From a928739456b78072136a1a264a68758571238aac Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 11 Nov 2024 04:07:54 +0800 Subject: [PATCH] Refactor sidebar assignee&milestone&project selectors (#32465) Follow #32460 Now the code could be much clearer than before and easier to maintain. A lot of legacy code is removed. Manually tested. This PR is large enough, that fine tunes could be deferred to the future if there is no bug found or design problem. Screenshots:
![image](https://github.com/user-attachments/assets/35f4ab7b-1bc0-4bad-a73c-a4569328303c)
--- modules/base/tool.go | 3 + modules/base/tool_test.go | 1 + modules/container/set.go | 4 +- modules/container/set_test.go | 2 + modules/templates/helper.go | 1 + routers/web/repo/compare.go | 12 +- routers/web/repo/issue.go | 474 +++++++++--------- routers/web/repo/pull.go | 2 +- services/forms/repo_form.go | 1 - .../repo/issue/milestone/select_menu.tmpl | 38 -- templates/repo/issue/new_form.tmpl | 138 +---- .../repo/issue/sidebar/assignee_list.tmpl | 69 ++- templates/repo/issue/sidebar/label_list.tmpl | 18 +- .../repo/issue/sidebar/label_list_item.tmpl | 2 +- .../repo/issue/sidebar/milestone_list.tmpl | 64 ++- .../repo/issue/sidebar/participant_list.tmpl | 2 +- .../repo/issue/sidebar/project_list.tmpl | 68 ++- .../repo/issue/sidebar/reviewer_list.tmpl | 24 +- .../repo/issue/view_content/sidebar.tmpl | 13 +- web_src/css/repo.css | 6 - .../features/repo-issue-sidebar-combolist.ts | 166 +++--- web_src/js/features/repo-issue-sidebar.md | 6 +- web_src/js/features/repo-issue-sidebar.ts | 219 +------- 23 files changed, 504 insertions(+), 829 deletions(-) delete mode 100644 templates/repo/issue/milestone/select_menu.tmpl diff --git a/modules/base/tool.go b/modules/base/tool.go index 9e43030f40..928c80700b 100644 --- a/modules/base/tool.go +++ b/modules/base/tool.go @@ -147,6 +147,9 @@ func StringsToInt64s(strs []string) ([]int64, error) { } ints := make([]int64, 0, len(strs)) for _, s := range strs { + if s == "" { + continue + } n, err := strconv.ParseInt(s, 10, 64) if err != nil { return nil, err diff --git a/modules/base/tool_test.go b/modules/base/tool_test.go index 4af8b9bc4d..86cccdf209 100644 --- a/modules/base/tool_test.go +++ b/modules/base/tool_test.go @@ -152,6 +152,7 @@ func TestStringsToInt64s(t *testing.T) { } testSuccess(nil, nil) testSuccess([]string{}, []int64{}) + testSuccess([]string{""}, []int64{}) testSuccess([]string{"-1234"}, []int64{-1234}) testSuccess([]string{"1", "4", "16", "64", "256"}, []int64{1, 4, 16, 64, 256}) diff --git a/modules/container/set.go b/modules/container/set.go index adb77dcac7..105533f203 100644 --- a/modules/container/set.go +++ b/modules/container/set.go @@ -31,8 +31,8 @@ func (s Set[T]) AddMultiple(values ...T) { } } -// Contains determines whether a set contains the specified elements. -// Returns true if the set contains the specified element; otherwise, false. +// Contains determines whether a set contains all these elements. +// Returns true if the set contains all these elements; otherwise, false. func (s Set[T]) Contains(values ...T) bool { ret := true for _, value := range values { diff --git a/modules/container/set_test.go b/modules/container/set_test.go index 1502236034..a8b7ff8190 100644 --- a/modules/container/set_test.go +++ b/modules/container/set_test.go @@ -18,7 +18,9 @@ func TestSet(t *testing.T) { assert.True(t, s.Contains("key1")) assert.True(t, s.Contains("key2")) + assert.True(t, s.Contains("key1", "key2")) assert.False(t, s.Contains("key3")) + assert.False(t, s.Contains("key1", "key3")) assert.True(t, s.Remove("key2")) assert.False(t, s.Contains("key2")) diff --git a/modules/templates/helper.go b/modules/templates/helper.go index efaa10624b..3ef11772dc 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -31,6 +31,7 @@ func NewFuncMap() template.FuncMap { "ctx": func() any { return nil }, // template context function "DumpVar": dumpVar, + "NIL": func() any { return nil }, // ----------------------------------------------------------------- // html/template related functions diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 9a7d3dfbf6..a5fdba3fde 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -788,19 +788,11 @@ func CompareDiff(ctx *context.Context) { if !nothingToCompare { // Setup information for new form. - retrieveRepoMetasForIssueWriter(ctx, ctx.Repo.Repository, true) + pageMetaData := retrieveRepoIssueMetaData(ctx, ctx.Repo.Repository, nil, true) if ctx.Written() { return } - labelsData := retrieveRepoLabels(ctx, ctx.Repo.Repository, 0, true) - if ctx.Written() { - return - } - RetrieveRepoReviewers(ctx, ctx.Repo.Repository, nil, true) - if ctx.Written() { - return - } - _, templateErrs := setTemplateIfExists(ctx, pullRequestTemplateKey, pullRequestTemplateCandidates, labelsData) + _, templateErrs := setTemplateIfExists(ctx, pullRequestTemplateKey, pullRequestTemplateCandidates, pageMetaData) if len(templateErrs) > 0 { ctx.Flash.Warning(renderErrorOfTemplates(ctx, templateErrs), true) } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index a4e2fd8cea..72f89bd27d 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -431,7 +431,7 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt return 0 } - retrieveProjects(ctx, repo) + retrieveProjectsForIssueList(ctx, repo) if ctx.Written() { return } @@ -556,37 +556,147 @@ 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) { +type issueSidebarMilestoneData struct { + SelectedMilestoneID int64 + OpenMilestones []*issues_model.Milestone + ClosedMilestones []*issues_model.Milestone +} + +type issueSidebarAssigneesData struct { + SelectedAssigneeIDs string + CandidateAssignees []*user_model.User +} + +type IssuePageMetaData struct { + RepoLink string + Repository *repo_model.Repository + Issue *issues_model.Issue + IsPullRequest bool + CanModifyIssueOrPull bool + + ReviewersData *issueSidebarReviewersData + LabelsData *issueSidebarLabelsData + MilestonesData *issueSidebarMilestoneData + ProjectsData *issueSidebarProjectsData + AssigneesData *issueSidebarAssigneesData +} + +func retrieveRepoIssueMetaData(ctx *context.Context, repo *repo_model.Repository, issue *issues_model.Issue, isPull bool) *IssuePageMetaData { + data := &IssuePageMetaData{ + RepoLink: ctx.Repo.RepoLink, + Repository: repo, + Issue: issue, + IsPullRequest: isPull, + + ReviewersData: &issueSidebarReviewersData{}, + LabelsData: &issueSidebarLabelsData{}, + MilestonesData: &issueSidebarMilestoneData{}, + ProjectsData: &issueSidebarProjectsData{}, + AssigneesData: &issueSidebarAssigneesData{}, + } + ctx.Data["IssuePageMetaData"] = data + + if isPull { + data.retrieveReviewersData(ctx) + if ctx.Written() { + return data + } + } + data.retrieveLabelsData(ctx) + if ctx.Written() { + return data + } + + data.CanModifyIssueOrPull = ctx.Repo.CanWriteIssuesOrPulls(isPull) && !ctx.Repo.Repository.IsArchived + if !data.CanModifyIssueOrPull { + return data + } + + data.retrieveAssigneesDataForIssueWriter(ctx) + if ctx.Written() { + return data + } + + data.retrieveMilestonesDataForIssueWriter(ctx) + if ctx.Written() { + return data + } + + data.retrieveProjectsDataForIssueWriter(ctx) + if ctx.Written() { + return data + } + + PrepareBranchList(ctx) + if ctx.Written() { + return data + } + + ctx.Data["CanCreateIssueDependencies"] = ctx.Repo.CanCreateIssueDependencies(ctx, ctx.Doer, isPull) + return data +} + +func (d *IssuePageMetaData) retrieveMilestonesDataForIssueWriter(ctx *context.Context) { var err error - ctx.Data["OpenMilestones"], err = db.Find[issues_model.Milestone](ctx, issues_model.FindMilestoneOptions{ - RepoID: repo.ID, + if d.Issue != nil { + d.MilestonesData.SelectedMilestoneID = d.Issue.MilestoneID + } + d.MilestonesData.OpenMilestones, err = db.Find[issues_model.Milestone](ctx, issues_model.FindMilestoneOptions{ + RepoID: d.Repository.ID, IsClosed: optional.Some(false), }) if err != nil { ctx.ServerError("GetMilestones", err) return } - ctx.Data["ClosedMilestones"], err = db.Find[issues_model.Milestone](ctx, issues_model.FindMilestoneOptions{ - RepoID: repo.ID, + d.MilestonesData.ClosedMilestones, err = db.Find[issues_model.Milestone](ctx, issues_model.FindMilestoneOptions{ + RepoID: d.Repository.ID, IsClosed: optional.Some(true), }) if err != nil { ctx.ServerError("GetMilestones", err) return } +} - assigneeUsers, err := repo_model.GetRepoAssignees(ctx, repo) +func (d *IssuePageMetaData) retrieveAssigneesDataForIssueWriter(ctx *context.Context) { + var err error + d.AssigneesData.CandidateAssignees, err = repo_model.GetRepoAssignees(ctx, d.Repository) if err != nil { ctx.ServerError("GetRepoAssignees", err) return } - ctx.Data["Assignees"] = shared_user.MakeSelfOnTop(ctx.Doer, assigneeUsers) - + d.AssigneesData.CandidateAssignees = shared_user.MakeSelfOnTop(ctx.Doer, d.AssigneesData.CandidateAssignees) + if d.Issue != nil { + _ = d.Issue.LoadAssignees(ctx) + ids := make([]string, 0, len(d.Issue.Assignees)) + for _, a := range d.Issue.Assignees { + ids = append(ids, strconv.FormatInt(a.ID, 10)) + } + d.AssigneesData.SelectedAssigneeIDs = strings.Join(ids, ",") + } + // FIXME: this is a tricky part which writes ctx.Data["Mentionable*"] handleTeamMentions(ctx) } -func retrieveProjects(ctx *context.Context, repo *repo_model.Repository) { +func retrieveProjectsForIssueList(ctx *context.Context, repo *repo_model.Repository) { + ctx.Data["OpenProjects"], ctx.Data["ClosedProjects"] = retrieveProjectsInternal(ctx, repo) +} + +type issueSidebarProjectsData struct { + SelectedProjectID int64 + OpenProjects []*project_model.Project + ClosedProjects []*project_model.Project +} + +func (d *IssuePageMetaData) retrieveProjectsDataForIssueWriter(ctx *context.Context) { + if d.Issue != nil && d.Issue.Project != nil { + d.ProjectsData.SelectedProjectID = d.Issue.Project.ID + } + d.ProjectsData.OpenProjects, d.ProjectsData.ClosedProjects = retrieveProjectsInternal(ctx, ctx.Repo.Repository) +} + +func retrieveProjectsInternal(ctx *context.Context, repo *repo_model.Repository) (open, closed []*project_model.Project) { // Distinguish whether the owner of the repository // is an individual or an organization repoOwnerType := project_model.TypeIndividual @@ -609,7 +719,7 @@ func retrieveProjects(ctx *context.Context, repo *repo_model.Repository) { }) if err != nil { ctx.ServerError("GetProjects", err) - return + return nil, nil } closedProjects, err = db.Find[project_model.Project](ctx, project_model.SearchOptions{ ListOptions: db.ListOptionsAll, @@ -619,7 +729,7 @@ func retrieveProjects(ctx *context.Context, repo *repo_model.Repository) { }) if err != nil { ctx.ServerError("GetProjects", err) - return + return nil, nil } } @@ -632,7 +742,7 @@ func retrieveProjects(ctx *context.Context, repo *repo_model.Repository) { }) if err != nil { ctx.ServerError("GetProjects", err) - return + return nil, nil } openProjects = append(openProjects, openProjects2...) closedProjects2, err := db.Find[project_model.Project](ctx, project_model.SearchOptions{ @@ -643,13 +753,11 @@ func retrieveProjects(ctx *context.Context, repo *repo_model.Repository) { }) if err != nil { ctx.ServerError("GetProjects", err) - return + return nil, nil } closedProjects = append(closedProjects, closedProjects2...) } - - ctx.Data["OpenProjects"] = openProjects - ctx.Data["ClosedProjects"] = closedProjects + return openProjects, closedProjects } // repoReviewerSelection items to bee shown @@ -665,10 +773,6 @@ type repoReviewerSelection struct { } type issueSidebarReviewersData struct { - Repository *repo_model.Repository - RepoOwnerName string - RepoLink string - IssueID int64 CanChooseReviewer bool OriginalReviews issues_model.ReviewList TeamReviewers []*repoReviewerSelection @@ -677,41 +781,44 @@ type issueSidebarReviewersData struct { } // 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 - data.Repository = repo - data.RepoOwnerName = repo.OwnerName - data.CanChooseReviewer = canChooseReviewer +func (d *IssuePageMetaData) retrieveReviewersData(ctx *context.Context) { + data := d.ReviewersData + repo := d.Repository + if ctx.Doer != nil && ctx.IsSigned { + if d.Issue == nil { + data.CanChooseReviewer = true + } else { + data.CanChooseReviewer = issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, repo, d.Issue) + } + } var posterID int64 var isClosed bool var reviews issues_model.ReviewList - if issue == nil { + if d.Issue == nil { posterID = ctx.Doer.ID } else { - posterID = issue.PosterID - if issue.OriginalAuthorID > 0 { + posterID = d.Issue.PosterID + if d.Issue.OriginalAuthorID > 0 { posterID = 0 // for migrated PRs, no poster ID } - data.IssueID = issue.ID - isClosed = issue.IsClosed || issue.PullRequest.HasMerged + isClosed = d.Issue.IsClosed || d.Issue.PullRequest.HasMerged - originalAuthorReviews, err := issues_model.GetReviewersFromOriginalAuthorsByIssueID(ctx, issue.ID) + originalAuthorReviews, err := issues_model.GetReviewersFromOriginalAuthorsByIssueID(ctx, d.Issue.ID) if err != nil { ctx.ServerError("GetReviewersFromOriginalAuthorsByIssueID", err) return } data.OriginalReviews = originalAuthorReviews - reviews, err = issues_model.GetReviewsByIssueID(ctx, issue.ID) + reviews, err = issues_model.GetReviewsByIssueID(ctx, d.Issue.ID) if err != nil { ctx.ServerError("GetReviewersByIssueID", err) return } - if len(reviews) == 0 && !canChooseReviewer { + if len(reviews) == 0 && !data.CanChooseReviewer { return } } @@ -724,7 +831,7 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is reviewers []*user_model.User ) - if canChooseReviewer { + if data.CanChooseReviewer { var err error reviewers, err = repo_model.GetReviewers(ctx, repo, ctx.Doer.ID, posterID) if err != nil { @@ -760,7 +867,7 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is tmp.ItemID = -review.ReviewerTeamID } - if canChooseReviewer { + if data.CanChooseReviewer { // Users who can choose reviewers can also remove review requests tmp.CanChange = true } else if ctx.Doer != nil && ctx.Doer.ID == review.ReviewerID && review.Type == issues_model.ReviewTypeRequest { @@ -770,7 +877,7 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is pullReviews = append(pullReviews, tmp) - if canChooseReviewer { + if data.CanChooseReviewer { if tmp.IsTeam { teamReviewersResult = append(teamReviewersResult, tmp) } else { @@ -811,7 +918,7 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is data.CurrentPullReviewers = currentPullReviewers } - if canChooseReviewer && reviewersResult != nil { + if data.CanChooseReviewer && reviewersResult != nil { preadded := len(reviewersResult) for _, reviewer := range reviewers { found := false @@ -839,7 +946,7 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is data.Reviewers = reviewersResult } - if canChooseReviewer && teamReviewersResult != nil { + if data.CanChooseReviewer && teamReviewersResult != nil { preadded := len(teamReviewersResult) for _, team := range teamReviewers { found := false @@ -866,15 +973,9 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is data.TeamReviewers = teamReviewersResult } - - ctx.Data["IssueSidebarReviewersData"] = data } type issueSidebarLabelsData struct { - Repository *repo_model.Repository - RepoLink string - IssueID int64 - IsPullRequest bool AllLabels []*issues_model.Label RepoLabels []*issues_model.Label OrgLabels []*issues_model.Label @@ -922,60 +1023,30 @@ func (d *issueSidebarLabelsData) SetSelectedLabelIDs(labelIDs []int64) { ) } -func retrieveRepoLabels(ctx *context.Context, repo *repo_model.Repository, issueID int64, isPull bool) *issueSidebarLabelsData { - labelsData := &issueSidebarLabelsData{ - Repository: repo, - RepoLink: ctx.Repo.RepoLink, - IssueID: issueID, - IsPullRequest: isPull, - } - ctx.Data["IssueSidebarLabelsData"] = labelsData +func (d *IssuePageMetaData) retrieveLabelsData(ctx *context.Context) { + repo := d.Repository + labelsData := d.LabelsData labels, err := issues_model.GetLabelsByRepoID(ctx, repo.ID, "", db.ListOptions{}) if err != nil { ctx.ServerError("GetLabelsByRepoID", err) - return nil + return } labelsData.RepoLabels = labels if repo.Owner.IsOrganization() { orgLabels, err := issues_model.GetLabelsByOrgID(ctx, repo.Owner.ID, ctx.FormString("sort"), db.ListOptions{}) if err != nil { - return nil + return } labelsData.OrgLabels = orgLabels } labelsData.AllLabels = append(labelsData.AllLabels, labelsData.RepoLabels...) labelsData.AllLabels = append(labelsData.AllLabels, labelsData.OrgLabels...) - return labelsData -} - -// retrieveRepoMetasForIssueWriter finds some the meta information of a repository for an issue/pr writer -func retrieveRepoMetasForIssueWriter(ctx *context.Context, repo *repo_model.Repository, isPull bool) { - if !ctx.Repo.CanWriteIssuesOrPulls(isPull) { - return - } - - RetrieveRepoMilestonesAndAssignees(ctx, repo) - if ctx.Written() { - return - } - - retrieveProjects(ctx, repo) - if ctx.Written() { - return - } - - PrepareBranchList(ctx) - if ctx.Written() { - return - } - // Contains true if the user can create issue dependencies - ctx.Data["CanCreateIssueDependencies"] = ctx.Repo.CanCreateIssueDependencies(ctx, ctx.Doer, isPull) } // 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, labelsData *issueSidebarLabelsData) (bool, map[string]error) { +func setTemplateIfExists(ctx *context.Context, ctxDataKey string, possibleFiles []string, metaData *IssuePageMetaData) (bool, map[string]error) { commit, err := ctx.Repo.GitRepo.GetBranchCommit(ctx.Repo.Repository.DefaultBranch) if err != nil { return false, nil @@ -1013,24 +1084,20 @@ func setTemplateIfExists(ctx *context.Context, ctxDataKey string, possibleFiles ctx.Data["TemplateFile"] = template.FileName } - labelsData.SetSelectedLabelNames(template.Labels) + metaData.LabelsData.SetSelectedLabelNames(template.Labels) - selectedAssigneeIDs := make([]int64, 0, len(template.Assignees)) selectedAssigneeIDStrings := make([]string, 0, len(template.Assignees)) - if userIDs, err := user_model.GetUserIDsByNames(ctx, template.Assignees, false); err == nil { + if userIDs, err := user_model.GetUserIDsByNames(ctx, template.Assignees, true); err == nil { for _, userID := range userIDs { - selectedAssigneeIDs = append(selectedAssigneeIDs, userID) selectedAssigneeIDStrings = append(selectedAssigneeIDStrings, strconv.FormatInt(userID, 10)) } } + metaData.AssigneesData.SelectedAssigneeIDs = strings.Join(selectedAssigneeIDStrings, ",") if template.Ref != "" && !strings.HasPrefix(template.Ref, "refs/") { // Assume that the ref intended is always a branch - for tags users should use refs/tags/ template.Ref = git.BranchPrefix + template.Ref } - ctx.Data["HasSelectedAssignee"] = len(selectedAssigneeIDs) > 0 - ctx.Data["assignee_ids"] = strings.Join(selectedAssigneeIDStrings, ",") - ctx.Data["SelectedAssigneeIDs"] = selectedAssigneeIDs ctx.Data["Reference"] = template.Ref ctx.Data["RefEndName"] = git.RefName(template.Ref).ShortName() return true, templateErrs @@ -1057,42 +1124,19 @@ func NewIssue(ctx *context.Context) { ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled upload.AddUploadContext(ctx, "comment") - milestoneID := ctx.FormInt64("milestone") - if milestoneID > 0 { - milestone, err := issues_model.GetMilestoneByRepoID(ctx, ctx.Repo.Repository.ID, milestoneID) - if err != nil { - log.Error("GetMilestoneByID: %d: %v", milestoneID, err) - } else { - ctx.Data["milestone_id"] = milestoneID - ctx.Data["Milestone"] = milestone - } + pageMetaData := retrieveRepoIssueMetaData(ctx, ctx.Repo.Repository, nil, false) + if ctx.Written() { + return } - projectID := ctx.FormInt64("project") - if projectID > 0 && isProjectsEnabled { - project, err := project_model.GetProjectByID(ctx, projectID) - if err != nil { - log.Error("GetProjectByID: %d: %v", projectID, err) - } else if project.RepoID != ctx.Repo.Repository.ID { - log.Error("GetProjectByID: %d: %v", projectID, fmt.Errorf("project[%d] not in repo [%d]", project.ID, ctx.Repo.Repository.ID)) - } else { - ctx.Data["project_id"] = projectID - ctx.Data["Project"] = project - } - + pageMetaData.MilestonesData.SelectedMilestoneID = ctx.FormInt64("milestone") + pageMetaData.ProjectsData.SelectedProjectID = ctx.FormInt64("project") + if pageMetaData.ProjectsData.SelectedProjectID > 0 { if len(ctx.Req.URL.Query().Get("project")) > 0 { ctx.Data["redirect_after_creation"] = "project" } } - retrieveRepoMetasForIssueWriter(ctx, ctx.Repo.Repository, false) - if ctx.Written() { - return - } - labelsData := retrieveRepoLabels(ctx, ctx.Repo.Repository, 0, false) - if ctx.Written() { - return - } tags, err := repo_model.GetTagNamesByRepoID(ctx, ctx.Repo.Repository.ID) if err != nil { ctx.ServerError("GetTagNamesByRepoID", err) @@ -1101,7 +1145,7 @@ func NewIssue(ctx *context.Context) { ctx.Data["Tags"] = tags ret := issue_service.ParseTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo) - templateLoaded, errs := setTemplateIfExists(ctx, issueTemplateKey, IssueTemplateCandidates, labelsData) + templateLoaded, errs := setTemplateIfExists(ctx, issueTemplateKey, IssueTemplateCandidates, pageMetaData) for k, v := range errs { ret.TemplateErrors[k] = v } @@ -1196,8 +1240,16 @@ func DeleteIssue(ctx *context.Context) { ctx.Redirect(fmt.Sprintf("%s/issues", ctx.Repo.Repository.Link()), http.StatusSeeOther) } -// ValidateRepoMetas check and returns repository's meta information -func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull bool) (ret struct { +func toSet[ItemType any, KeyType comparable](slice []ItemType, keyFunc func(ItemType) KeyType) container.Set[KeyType] { + s := make(container.Set[KeyType]) + for _, item := range slice { + s.Add(keyFunc(item)) + } + return s +} + +// ValidateRepoMetasForNewIssue check and returns repository's meta information +func ValidateRepoMetasForNewIssue(ctx *context.Context, form forms.CreateIssueForm, isPull bool) (ret struct { LabelIDs, AssigneeIDs []int64 MilestoneID, ProjectID int64 @@ -1205,126 +1257,76 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull TeamReviewers []*organization.Team }, ) { - var ( - repo = ctx.Repo.Repository - err error - ) - - retrieveRepoMetasForIssueWriter(ctx, ctx.Repo.Repository, isPull) - if ctx.Written() { - return ret - } - labelsData := retrieveRepoLabels(ctx, ctx.Repo.Repository, 0, isPull) + pageMetaData := retrieveRepoIssueMetaData(ctx, ctx.Repo.Repository, nil, isPull) if ctx.Written() { return ret } - var labelIDs []int64 - // Check labels. - if len(form.LabelIDs) > 0 { - labelIDs, err = base.StringsToInt64s(strings.Split(form.LabelIDs, ",")) - if err != nil { - return ret - } - labelsData.SetSelectedLabelIDs(labelIDs) - } - - // Check milestone. - milestoneID := form.MilestoneID - if milestoneID > 0 { - milestone, err := issues_model.GetMilestoneByRepoID(ctx, ctx.Repo.Repository.ID, milestoneID) - if err != nil { - ctx.ServerError("GetMilestoneByID", err) - return ret - } - if milestone.RepoID != repo.ID { - ctx.ServerError("GetMilestoneByID", err) - return ret - } - ctx.Data["Milestone"] = milestone - ctx.Data["milestone_id"] = milestoneID + inputLabelIDs, _ := base.StringsToInt64s(strings.Split(form.LabelIDs, ",")) + candidateLabels := toSet(pageMetaData.LabelsData.AllLabels, func(label *issues_model.Label) int64 { return label.ID }) + if len(inputLabelIDs) > 0 && !candidateLabels.Contains(inputLabelIDs...) { + ctx.NotFound("", nil) + return ret } + pageMetaData.LabelsData.SetSelectedLabelIDs(inputLabelIDs) - if form.ProjectID > 0 { - p, err := project_model.GetProjectByID(ctx, form.ProjectID) - if err != nil { - ctx.ServerError("GetProjectByID", err) - return ret - } - if p.RepoID != ctx.Repo.Repository.ID && p.OwnerID != ctx.Repo.Repository.OwnerID { - ctx.NotFound("", nil) - return ret - } - - ctx.Data["Project"] = p - ctx.Data["project_id"] = form.ProjectID + allMilestones := append(slices.Clone(pageMetaData.MilestonesData.OpenMilestones), pageMetaData.MilestonesData.ClosedMilestones...) + candidateMilestones := toSet(allMilestones, func(milestone *issues_model.Milestone) int64 { return milestone.ID }) + if form.MilestoneID > 0 && !candidateMilestones.Contains(form.MilestoneID) { + ctx.NotFound("", nil) + return ret } + pageMetaData.MilestonesData.SelectedMilestoneID = form.MilestoneID - // Check assignees - var assigneeIDs []int64 - if len(form.AssigneeIDs) > 0 { - assigneeIDs, err = base.StringsToInt64s(strings.Split(form.AssigneeIDs, ",")) - if err != nil { - return ret - } - - // Check if the passed assignees actually exists and is assignable - for _, aID := range assigneeIDs { - assignee, err := user_model.GetUserByID(ctx, aID) - if err != nil { - ctx.ServerError("GetUserByID", err) - return ret - } - - valid, err := access_model.CanBeAssigned(ctx, assignee, repo, isPull) - if err != nil { - ctx.ServerError("CanBeAssigned", err) - return ret - } - - if !valid { - ctx.ServerError("canBeAssigned", repo_model.ErrUserDoesNotHaveAccessToRepo{UserID: aID, RepoName: repo.Name}) - return ret - } - } + allProjects := append(slices.Clone(pageMetaData.ProjectsData.OpenProjects), pageMetaData.ProjectsData.ClosedProjects...) + candidateProjects := toSet(allProjects, func(project *project_model.Project) int64 { return project.ID }) + if form.ProjectID > 0 && !candidateProjects.Contains(form.ProjectID) { + ctx.NotFound("", nil) + return ret } + pageMetaData.ProjectsData.SelectedProjectID = form.ProjectID - // Keep the old assignee id thingy for compatibility reasons - if form.AssigneeID > 0 { - assigneeIDs = append(assigneeIDs, form.AssigneeID) + candidateAssignees := toSet(pageMetaData.AssigneesData.CandidateAssignees, func(user *user_model.User) int64 { return user.ID }) + inputAssigneeIDs, _ := base.StringsToInt64s(strings.Split(form.AssigneeIDs, ",")) + if len(inputAssigneeIDs) > 0 && !candidateAssignees.Contains(inputAssigneeIDs...) { + ctx.NotFound("", nil) + return ret } + pageMetaData.AssigneesData.SelectedAssigneeIDs = form.AssigneeIDs - // Check reviewers + // Check if the passed reviewers (user/team) actually exist var reviewers []*user_model.User var teamReviewers []*organization.Team - if isPull && len(form.ReviewerIDs) > 0 { - reviewerIDs, err := base.StringsToInt64s(strings.Split(form.ReviewerIDs, ",")) - if err != nil { - return ret + reviewerIDs, _ := base.StringsToInt64s(strings.Split(form.ReviewerIDs, ",")) + if isPull && len(reviewerIDs) > 0 { + userReviewersMap := map[int64]*user_model.User{} + teamReviewersMap := map[int64]*organization.Team{} + for _, r := range pageMetaData.ReviewersData.Reviewers { + userReviewersMap[r.User.ID] = r.User + } + for _, r := range pageMetaData.ReviewersData.TeamReviewers { + teamReviewersMap[r.Team.ID] = r.Team } - // Check if the passed reviewers (user/team) actually exist for _, rID := range reviewerIDs { - // negative reviewIDs represent team requests - if rID < 0 { - teamReviewer, err := organization.GetTeamByID(ctx, -rID) - if err != nil { - ctx.ServerError("GetTeamByID", err) + if rID < 0 { // negative reviewIDs represent team requests + team, ok := teamReviewersMap[-rID] + if !ok { + ctx.NotFound("", nil) return ret } - teamReviewers = append(teamReviewers, teamReviewer) - continue - } - - reviewer, err := user_model.GetUserByID(ctx, rID) - if err != nil { - ctx.ServerError("GetUserByID", err) - return ret + teamReviewers = append(teamReviewers, team) + } else { + user, ok := userReviewersMap[rID] + if !ok { + ctx.NotFound("", nil) + return ret + } + reviewers = append(reviewers, user) } - reviewers = append(reviewers, reviewer) } } - ret.LabelIDs, ret.AssigneeIDs, ret.MilestoneID, ret.ProjectID = labelIDs, assigneeIDs, milestoneID, form.ProjectID + ret.LabelIDs, ret.AssigneeIDs, ret.MilestoneID, ret.ProjectID = inputLabelIDs, inputAssigneeIDs, form.MilestoneID, form.ProjectID ret.Reviewers, ret.TeamReviewers = reviewers, teamReviewers return ret } @@ -1344,7 +1346,7 @@ func NewIssuePost(ctx *context.Context) { attachments []string ) - validateRet := ValidateRepoMetas(ctx, *form, false) + validateRet := ValidateRepoMetasForNewIssue(ctx, *form, false) if ctx.Written() { return } @@ -1619,37 +1621,11 @@ func ViewIssue(ctx *context.Context) { } } - retrieveRepoMetasForIssueWriter(ctx, repo, issue.IsPull) + pageMetaData := retrieveRepoIssueMetaData(ctx, repo, issue, issue.IsPull) if ctx.Written() { return } - labelsData := retrieveRepoLabels(ctx, repo, issue.ID, issue.IsPull) - if ctx.Written() { - return - } - labelsData.SetSelectedLabels(issue.Labels) - - // Check milestone and assignee. - if ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) { - RetrieveRepoMilestonesAndAssignees(ctx, repo) - retrieveProjects(ctx, repo) - - if ctx.Written() { - return - } - } - - if issue.IsPull { - canChooseReviewer := false - if ctx.Doer != nil && ctx.IsSigned { - canChooseReviewer = issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, repo, issue) - } - - RetrieveRepoReviewers(ctx, repo, issue, canChooseReviewer) - if ctx.Written() { - return - } - } + pageMetaData.LabelsData.SetSelectedLabels(issue.Labels) if ctx.IsSigned { // Update issue-user. diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index dd9671efbe..bb814eab6e 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1269,7 +1269,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { return } - validateRet := ValidateRepoMetas(ctx, *form, true) + validateRet := ValidateRepoMetasForNewIssue(ctx, *form, true) if ctx.Written() { return } diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 83f2dd6caa..d27bbca894 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -451,7 +451,6 @@ type CreateIssueForm struct { Ref string `form:"ref"` MilestoneID int64 ProjectID int64 - AssigneeID int64 Content string Files []string AllowMaintainerEdit bool diff --git a/templates/repo/issue/milestone/select_menu.tmpl b/templates/repo/issue/milestone/select_menu.tmpl deleted file mode 100644 index 9b0492ce52..0000000000 --- a/templates/repo/issue/milestone/select_menu.tmpl +++ /dev/null @@ -1,38 +0,0 @@ -{{if or .OpenMilestones .ClosedMilestones}} -
- {{svg "octicon-search" 16}} - -
-
-{{end}} -
{{ctx.Locale.Tr "repo.issues.new.clear_milestone"}}
-{{if and (not .OpenMilestones) (not .ClosedMilestones)}} -
- {{ctx.Locale.Tr "repo.issues.new.no_items"}} -
-{{else}} - {{if .OpenMilestones}} -
-
- {{ctx.Locale.Tr "repo.issues.new.open_milestone"}} -
- {{range .OpenMilestones}} - - {{svg "octicon-milestone" 16 "tw-mr-1"}} - {{.Name}} - - {{end}} - {{end}} - {{if .ClosedMilestones}} -
-
- {{ctx.Locale.Tr "repo.issues.new.closed_milestone"}} -
- {{range .ClosedMilestones}} - - {{svg "octicon-milestone" 16 "tw-mr-1"}} - {{.Name}} - - {{end}} - {{end}} -{{end}} diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index 65d359e9dc..ceaaebc4d5 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -49,142 +49,22 @@
{{template "repo/issue/branch_selector_field" $}} {{if .PageIsComparePull}} - {{template "repo/issue/sidebar/reviewer_list" $.IssueSidebarReviewersData}} + {{template "repo/issue/sidebar/reviewer_list" $.IssuePageMetaData}}
{{end}} - {{template "repo/issue/sidebar/label_list" $.IssueSidebarLabelsData}} - -
- - -
- - {{ctx.Locale.Tr "repo.issues.new.milestone"}} - {{if .HasIssuesOrPullsWritePermission}} - {{svg "octicon-gear" 16 "tw-ml-1"}} - {{end}} - -
- {{template "repo/issue/milestone/select_menu" .}} -
-
-
- {{ctx.Locale.Tr "repo.issues.new.no_milestone"}} -
- {{if .Milestone}} - - {{svg "octicon-milestone" 18 "tw-mr-2"}} - {{.Milestone.Name}} - - {{end}} -
-
- + {{template "repo/issue/sidebar/label_list" $.IssuePageMetaData}} + {{template "repo/issue/sidebar/milestone_list" $.IssuePageMetaData}} {{if .IsProjectsEnabled}} -
- - -
- - {{ctx.Locale.Tr "repo.issues.new.projects"}} - {{if .HasIssuesOrPullsWritePermission}} - {{svg "octicon-gear" 16 "tw-ml-1"}} - {{end}} - -
- {{if or .OpenProjects .ClosedProjects}} -
- {{svg "octicon-search" 16}} - -
- {{end}} -
{{ctx.Locale.Tr "repo.issues.new.clear_projects"}}
- {{if and (not .OpenProjects) (not .ClosedProjects)}} -
- {{ctx.Locale.Tr "repo.issues.new.no_items"}} -
- {{else}} - {{if .OpenProjects}} -
-
- {{ctx.Locale.Tr "repo.issues.new.open_projects"}} -
- {{range .OpenProjects}} - - {{svg .IconName 18 "tw-mr-2"}}{{.Title}} - - {{end}} - {{end}} - {{if .ClosedProjects}} -
-
- {{ctx.Locale.Tr "repo.issues.new.closed_projects"}} -
- {{range .ClosedProjects}} - - {{svg .IconName 18 "tw-mr-2"}}{{.Title}} - - {{end}} - {{end}} - {{end}} -
-
-
- {{ctx.Locale.Tr "repo.issues.new.no_projects"}} -
- {{if .Project}} - - {{svg .Project.IconName 18 "tw-mr-2"}}{{.Project.Title}} - - {{end}} -
-
+ {{template "repo/issue/sidebar/project_list" $.IssuePageMetaData}} {{end}} -
- -
- - {{ctx.Locale.Tr "repo.issues.new.assignees"}} - {{if .HasIssuesOrPullsWritePermission}} - {{svg "octicon-gear" 16 "tw-ml-1"}} - {{end}} - -
-
- {{svg "octicon-search" 16}} - -
-
{{ctx.Locale.Tr "repo.issues.new.clear_assignees"}}
- {{range .Assignees}} - - {{svg "octicon-check"}} - - {{ctx.AvatarUtils.Avatar . 28 "tw-mr-2"}}{{template "repo/search_name" .}} - - - {{end}} -
-
-
- - {{ctx.Locale.Tr "repo.issues.new.no_assignees"}} - -
- {{range .Assignees}} - - {{ctx.AvatarUtils.Avatar . 28 "tw-mr-2 tw-align-middle"}}{{.GetDisplayName}} - - {{end}} -
-
+ {{template "repo/issue/sidebar/assignee_list" $.IssuePageMetaData}} + {{if and .PageIsComparePull (not (eq .HeadRepo.FullName .BaseCompareRepo.FullName)) .CanWriteToHeadRepo}}
-
-
- - -
+
+ +
{{end}}
diff --git a/templates/repo/issue/sidebar/assignee_list.tmpl b/templates/repo/issue/sidebar/assignee_list.tmpl index 260f7c5be4..bee6123e52 100644 --- a/templates/repo/issue/sidebar/assignee_list.tmpl +++ b/templates/repo/issue/sidebar/assignee_list.tmpl @@ -1,46 +1,35 @@ +{{$pageMeta := .}} +{{$data := .AssigneesData}} +{{$issueAssignees := NIL}}{{if $pageMeta.Issue}}{{$issueAssignees = $pageMeta.Issue.Assignees}}{{end}}
- -
- - {{ctx.Locale.Tr "repo.issues.new.assignees"}} - {{if and .HasIssuesOrPullsWritePermission (not .Repository.IsArchived)}} - {{svg "octicon-gear" 16 "tw-ml-1"}} - {{end}} - -
-
- {{svg "octicon-search" 16}} - +
+ +
+ + {{ctx.Locale.Tr "repo.issues.new.assignees"}} {{if $pageMeta.CanModifyIssueOrPull}}{{svg "octicon-gear"}}{{end}} + +
+
+ {{svg "octicon-search" 16}} + +
+
{{ctx.Locale.Tr "repo.issues.new.clear_assignees"}}
+ {{range $data.CandidateAssignees}} + + {{svg "octicon-check"}} + {{ctx.AvatarUtils.Avatar . 20}} {{template "repo/search_name" .}} + + {{end}}
-
{{ctx.Locale.Tr "repo.issues.new.clear_assignees"}}
- {{range .Assignees}} - - {{$AssigneeID := .ID}} - - {{$checked := false}} - {{range $.Issue.Assignees}} - {{if eq .ID $AssigneeID}} - {{$checked = true}} - {{end}} - {{end}} - {{svg "octicon-check"}} - - {{ctx.AvatarUtils.Avatar . 20 "tw-mr-2"}}{{template "repo/search_name" .}} - - - {{end}}
-
-
- {{ctx.Locale.Tr "repo.issues.new.no_assignees"}} -
- {{range .Issue.Assignees}} -
- - {{ctx.AvatarUtils.Avatar . 28 "tw-mr-2"}} - {{.GetDisplayName}} - -
+
+ {{ctx.Locale.Tr "repo.issues.new.no_assignees"}} + {{range $issueAssignees}} + + {{ctx.AvatarUtils.Avatar . 20}} {{.GetDisplayName}} + {{end}}
diff --git a/templates/repo/issue/sidebar/label_list.tmpl b/templates/repo/issue/sidebar/label_list.tmpl index e9f4baa433..ed80047661 100644 --- a/templates/repo/issue/sidebar/label_list.tmpl +++ b/templates/repo/issue/sidebar/label_list.tmpl @@ -1,10 +1,12 @@ -{{$data := .}} -{{$canChange := and ctx.RootData.HasIssuesOrPullsWritePermission (not $data.Repository.IsArchived)}} -
+{{$pageMeta := .}} +{{$data := .LabelsData}} +
-
+
- {{ctx.Locale.Tr "repo.issues.new.labels"}} {{if $canChange}}{{svg "octicon-gear"}}{{end}} + {{ctx.Locale.Tr "repo.issues.new.labels"}} {{if $pageMeta.CanModifyIssueOrPull}}{{svg "octicon-gear"}}{{end}}
{{if not $data.AllLabels}} @@ -16,7 +18,7 @@
{{ctx.Locale.Tr "repo.issues.new.clear_labels"}} {{$previousExclusiveScope := "_no_scope"}} - {{range .RepoLabels}} + {{range $data.RepoLabels}} {{$exclusiveScope := .ExclusiveScope}} {{if and (ne $previousExclusiveScope "_no_scope") (ne $previousExclusiveScope $exclusiveScope)}}
@@ -26,7 +28,7 @@ {{end}}
{{$previousExclusiveScope = "_no_scope"}} - {{range .OrgLabels}} + {{range $data.OrgLabels}} {{$exclusiveScope := .ExclusiveScope}} {{if and (ne $previousExclusiveScope "_no_scope") (ne $previousExclusiveScope $exclusiveScope)}}
@@ -42,7 +44,7 @@ {{ctx.Locale.Tr "repo.issues.new.no_label"}} {{range $data.AllLabels}} {{if .IsChecked}} - + {{- ctx.RenderUtils.RenderLabel . -}} {{end}} diff --git a/templates/repo/issue/sidebar/label_list_item.tmpl b/templates/repo/issue/sidebar/label_list_item.tmpl index ad878e918b..5c6808d95b 100644 --- a/templates/repo/issue/sidebar/label_list_item.tmpl +++ b/templates/repo/issue/sidebar/label_list_item.tmpl @@ -1,5 +1,5 @@ {{$label := .Label}} - {{svg (Iif $label.ExclusiveScope "octicon-dot-fill" "octicon-check")}} diff --git a/templates/repo/issue/sidebar/milestone_list.tmpl b/templates/repo/issue/sidebar/milestone_list.tmpl index e9ca02f77a..4f2b4cb06f 100644 --- a/templates/repo/issue/sidebar/milestone_list.tmpl +++ b/templates/repo/issue/sidebar/milestone_list.tmpl @@ -1,22 +1,52 @@ +{{$pageMeta := .}} +{{$data := .MilestonesData}} +{{$issueMilestone := NIL}}{{if and $pageMeta.Issue $pageMeta.Issue.Milestone}}{{$issueMilestone = $pageMeta.Issue.Milestone}}{{end}}
-
- - {{ctx.Locale.Tr "repo.issues.new.milestone"}} - {{if and .HasIssuesOrPullsWritePermission (not .Repository.IsArchived)}} - {{svg "octicon-gear" 16 "tw-ml-1"}} - {{end}} - -
- {{template "repo/issue/milestone/select_menu" .}} +
+ +
+ + {{ctx.Locale.Tr "repo.issues.new.milestone"}} {{if $pageMeta.CanModifyIssueOrPull}}{{svg "octicon-gear"}}{{end}} + +
+ {{if and (not $data.OpenMilestones) (not $data.ClosedMilestones)}} +
{{ctx.Locale.Tr "repo.issues.new.no_items"}}
+ {{else}} +
+ {{svg "octicon-search"}} + +
+
+
{{ctx.Locale.Tr "repo.issues.new.clear_milestone"}}
+ {{if $data.OpenMilestones}} +
+
{{ctx.Locale.Tr "repo.issues.new.open_milestone"}}
+ {{range $data.OpenMilestones}} + + {{svg "octicon-milestone" 18}} {{.Name}} + + {{end}} + {{end}} + {{if $data.ClosedMilestones}} +
+
{{ctx.Locale.Tr "repo.issues.new.closed_milestone"}}
+ {{range $data.ClosedMilestones}} + + {{svg "octicon-milestone" 18}} {{.Name}} + + {{end}} + {{end}} + {{end}} +
-
-
- {{ctx.Locale.Tr "repo.issues.new.no_milestone"}} -
- {{if .Issue.Milestone}} - - {{svg "octicon-milestone" 18 "tw-mr-2"}} - {{.Issue.Milestone.Name}} + +
+ {{ctx.Locale.Tr "repo.issues.new.no_milestone"}} + {{if $issueMilestone}} + + {{svg "octicon-milestone" 18}} {{$issueMilestone.Name}} {{end}}
diff --git a/templates/repo/issue/sidebar/participant_list.tmpl b/templates/repo/issue/sidebar/participant_list.tmpl index 91c36fc01e..11debf95c4 100644 --- a/templates/repo/issue/sidebar/participant_list.tmpl +++ b/templates/repo/issue/sidebar/participant_list.tmpl @@ -4,7 +4,7 @@
{{range .Participants}} - {{ctx.AvatarUtils.Avatar . 28 "tw-my-0.5 tw-mr-1"}} + {{ctx.AvatarUtils.Avatar . 20 "tw-my-0.5 tw-mr-1"}} {{end}}
diff --git a/templates/repo/issue/sidebar/project_list.tmpl b/templates/repo/issue/sidebar/project_list.tmpl index ec79f8032f..ab1243cadd 100644 --- a/templates/repo/issue/sidebar/project_list.tmpl +++ b/templates/repo/issue/sidebar/project_list.tmpl @@ -1,53 +1,49 @@ -{{if .IsProjectsEnabled}} -
- -
- - {{ctx.Locale.Tr "repo.issues.new.projects"}} - {{if and .HasIssuesOrPullsWritePermission (not .Repository.IsArchived)}} - {{svg "octicon-gear" 16 "tw-ml-1"}} - {{end}} +{{$pageMeta := .}} +{{$data := .ProjectsData}} +{{$issueProject := NIL}}{{if and $pageMeta.Issue $pageMeta.Issue.Project}}{{$issueProject = $pageMeta.Issue.Project}}{{end}} +
+
+ +
+ + {{ctx.Locale.Tr "repo.issues.new.projects"}} {{if $pageMeta.CanModifyIssueOrPull}}{{svg "octicon-gear"}}{{end}} -
- {{if or .OpenProjects .ClosedProjects}} +
+ {{if or $data.OpenProjects $data.ClosedProjects}}
{{svg "octicon-search" 16}}
{{end}} -
{{ctx.Locale.Tr "repo.issues.new.clear_projects"}}
- {{if .OpenProjects}} +
{{ctx.Locale.Tr "repo.issues.new.clear_projects"}}
+ {{if $data.OpenProjects}}
-
- {{ctx.Locale.Tr "repo.issues.new.open_projects"}} -
- {{range .OpenProjects}} - - {{svg .IconName 18 "tw-mr-2"}}{{.Title}} +
{{ctx.Locale.Tr "repo.issues.new.open_projects"}}
+ {{range $data.OpenProjects}} + + {{svg .IconName 18}} {{.Title}} {{end}} {{end}} - {{if .ClosedProjects}} + {{if $data.ClosedProjects}}
-
- {{ctx.Locale.Tr "repo.issues.new.closed_projects"}} -
- {{range .ClosedProjects}} - - {{svg .IconName 18 "tw-mr-2"}}{{.Title}} +
{{ctx.Locale.Tr "repo.issues.new.closed_projects"}}
+ {{range $data.ClosedProjects}} + + {{svg .IconName 18}} {{.Title}} {{end}} {{end}}
-
- {{ctx.Locale.Tr "repo.issues.new.no_projects"}} -
- {{if .Issue.Project}} - - {{svg .Issue.Project.IconName 18 "tw-mr-2"}}{{.Issue.Project.Title}} - - {{end}} -
+
+ {{ctx.Locale.Tr "repo.issues.new.no_projects"}} + {{if $issueProject}} + + {{svg $issueProject.IconName 18}} {{$issueProject.Title}} + + {{end}}
-{{end}} +
diff --git a/templates/repo/issue/sidebar/reviewer_list.tmpl b/templates/repo/issue/sidebar/reviewer_list.tmpl index cf7b97c02b..e990fc5afc 100644 --- a/templates/repo/issue/sidebar/reviewer_list.tmpl +++ b/templates/repo/issue/sidebar/reviewer_list.tmpl @@ -1,10 +1,14 @@ -{{$data := .}} +{{$pageMeta := .}} +{{$data := .ReviewersData}} +{{$repoOwnerName := $pageMeta.Repository.OwnerName}} {{$hasCandidates := or $data.Reviewers $data.TeamReviewers}} -
+
{{/* match CreateIssueForm */}}
- {{ctx.Locale.Tr "repo.issues.review.reviewers"}} {{if and $data.CanChooseReviewer}}{{svg "octicon-gear"}}{{end}} + {{ctx.Locale.Tr "repo.issues.review.reviewers"}} {{if $data.CanChooseReviewer}}{{svg "octicon-gear"}}{{end}}
{{if $hasCandidates}} @@ -29,7 +33,7 @@ {{svg "octicon-check"}} - {{svg "octicon-people" 20}} {{$data.RepoOwnerName}}/{{.Team.Name}} + {{svg "octicon-people" 20}} {{$repoOwnerName}}/{{.Team.Name}} {{end}} {{end}} @@ -47,7 +51,7 @@ {{if .User}} {{ctx.AvatarUtils.Avatar .User 20}} {{.User.GetDisplayName}} {{else if .Team}} - {{svg "octicon-people" 20}} {{$data.RepoOwnerName}}/{{.Team.Name}} + {{svg "octicon-people" 20}} {{$repoOwnerName}}/{{.Team.Name}} {{end}}
@@ -64,13 +68,13 @@ {{if .Requested}} + data-url="{{$pageMeta.RepoLink}}/issues/request_review?action=detach&issue_ids={{$pageMeta.Issue.ID}}&id={{.ItemID}}"> {{svg "octicon-trash"}} {{else}} + data-url="{{$pageMeta.RepoLink}}/issues/request_review?action=attach&issue_ids={{$pageMeta.Issue.ID}}&id={{.ItemID}}"> {{svg "octicon-sync"}} {{end}} @@ -84,8 +88,8 @@ {{range $data.OriginalReviews}}
- {{$originalURLHostname := $data.Repository.GetOriginalURLHostname}} - {{$originalURL := $data.Repository.OriginalURL}} + {{$originalURLHostname := $pageMeta.Repository.GetOriginalURLHostname}} + {{$originalURL := $pageMeta.Repository.OriginalURL}} {{svg (MigrationIcon $originalURLHostname) 20}} {{.OriginalAuthor}} @@ -108,7 +112,7 @@
{{ctx.Locale.Tr "repo.issues.dismiss_review_warning"}}
-
+ {{ctx.RootData.CsrfTokenHtml}}
diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index 0fae1e9e1c..02f5d3e2df 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -2,16 +2,19 @@ {{template "repo/issue/branch_selector_field" $}} {{if .Issue.IsPull}} - {{template "repo/issue/sidebar/reviewer_list" $.IssueSidebarReviewersData}} + {{template "repo/issue/sidebar/reviewer_list" $.IssuePageMetaData}} {{template "repo/issue/sidebar/wip_switch" $}}
{{end}} - {{template "repo/issue/sidebar/label_list" $.IssueSidebarLabelsData}} + {{template "repo/issue/sidebar/label_list" $.IssuePageMetaData}} + + {{template "repo/issue/sidebar/milestone_list" $.IssuePageMetaData}} + {{if .IsProjectsEnabled}} + {{template "repo/issue/sidebar/project_list" $.IssuePageMetaData}} + {{end}} + {{template "repo/issue/sidebar/assignee_list" $.IssuePageMetaData}} - {{template "repo/issue/sidebar/milestone_list" $}} - {{template "repo/issue/sidebar/project_list" $}} - {{template "repo/issue/sidebar/assignee_list" $}} {{template "repo/issue/sidebar/participant_list" $}} {{template "repo/issue/sidebar/watch_notification" $}} {{template "repo/issue/sidebar/stopwatch_timetracker" $}} diff --git a/web_src/css/repo.css b/web_src/css/repo.css index ff8342d29a..01ddab97e5 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -2453,12 +2453,6 @@ tbody.commit-list { margin-top: 1em; } -.sidebar-item-link { - display: inline-flex; - align-items: center; - overflow-wrap: anywhere; -} - .diff-file-header { padding: 5px 8px !important; box-shadow: 0 -1px 0 1px var(--color-body); /* prevent borders being visible behind top corners when sticky and scrolled */ diff --git a/web_src/js/features/repo-issue-sidebar-combolist.ts b/web_src/js/features/repo-issue-sidebar-combolist.ts index f408eb43ba..24d620547f 100644 --- a/web_src/js/features/repo-issue-sidebar-combolist.ts +++ b/web_src/js/features/repo-issue-sidebar-combolist.ts @@ -3,7 +3,7 @@ import {POST} from '../modules/fetch.ts'; import {queryElemChildren, queryElems, toggleElem} from '../utils/dom.ts'; // if there are draft comments, confirm before reloading, to avoid losing comments -export function issueSidebarReloadConfirmDraftComment() { +function issueSidebarReloadConfirmDraftComment() { const commentTextareas = [ document.querySelector('.edit-content-zone:not(.tw-hidden) textarea'), document.querySelector('#comment-form textarea'), @@ -22,84 +22,138 @@ export function issueSidebarReloadConfirmDraftComment() { window.location.reload(); } -function collectCheckedValues(elDropdown: HTMLElement) { - return Array.from(elDropdown.querySelectorAll('.menu > .item.checked'), (el) => el.getAttribute('data-value')); -} +class IssueSidebarComboList { + updateUrl: string; + updateAlgo: string; + selectionMode: string; + elDropdown: HTMLElement; + elList: HTMLElement; + elComboValue: HTMLInputElement; + initialValues: string[]; -export function initIssueSidebarComboList(container: HTMLElement) { - 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'); - let initialValues = collectCheckedValues(elDropdown); + constructor(private container: HTMLElement) { + this.updateUrl = this.container.getAttribute('data-update-url'); + this.updateAlgo = container.getAttribute('data-update-algo'); + this.selectionMode = container.getAttribute('data-selection-mode'); + if (!['single', 'multiple'].includes(this.selectionMode)) throw new Error(`Invalid data-update-on: ${this.selectionMode}`); + if (!['diff', 'all'].includes(this.updateAlgo)) throw new Error(`Invalid data-update-algo: ${this.updateAlgo}`); + this.elDropdown = container.querySelector(':scope > .ui.dropdown'); + this.elList = container.querySelector(':scope > .ui.list'); + this.elComboValue = container.querySelector(':scope > .combo-value'); + } + + collectCheckedValues() { + return Array.from(this.elDropdown.querySelectorAll('.menu > .item.checked'), (el) => el.getAttribute('data-value')); + } + + updateUiList(changedValues) { + const elEmptyTip = this.elList.querySelector('.item.empty-list'); + queryElemChildren(this.elList, '.item:not(.empty-list)', (el) => el.remove()); + for (const value of changedValues) { + const el = this.elDropdown.querySelector(`.menu > .item[data-value="${CSS.escape(value)}"]`); + if (!el) continue; + const listItem = el.cloneNode(true) as HTMLElement; + queryElems(listItem, '.item-check-mark, .item-secondary-info', (el) => el.remove()); + this.elList.append(listItem); + } + const hasItems = Boolean(this.elList.querySelector('.item:not(.empty-list)')); + toggleElem(elEmptyTip, !hasItems); + } + + async updateToBackend(changedValues) { + if (this.updateAlgo === 'diff') { + for (const value of this.initialValues) { + if (!changedValues.includes(value)) { + await POST(this.updateUrl, {data: new URLSearchParams({action: 'detach', id: value})}); + } + } + for (const value of changedValues) { + if (!this.initialValues.includes(value)) { + await POST(this.updateUrl, {data: new URLSearchParams({action: 'attach', id: value})}); + } + } + } else { + await POST(this.updateUrl, {data: new URLSearchParams({id: changedValues.join(',')})}); + } + issueSidebarReloadConfirmDraftComment(); + } + + async doUpdate() { + const changedValues = this.collectCheckedValues(); + if (this.initialValues.join(',') === changedValues.join(',')) return; + this.updateUiList(changedValues); + if (this.updateUrl) await this.updateToBackend(changedValues); + this.initialValues = changedValues; + } + + async onChange() { + if (this.selectionMode === 'single') { + await this.doUpdate(); + fomanticQuery(this.elDropdown).dropdown('hide'); + } + } - elDropdown.addEventListener('click', (e) => { + async onItemClick(e) { const elItem = (e.target as HTMLElement).closest('.item'); if (!elItem) return; e.preventDefault(); if (elItem.hasAttribute('data-can-change') && elItem.getAttribute('data-can-change') !== 'true') return; if (elItem.matches('.clear-selection')) { - queryElems(elDropdown, '.menu > .item', (el) => el.classList.remove('checked')); - elComboValue.value = ''; + queryElems(this.elDropdown, '.menu > .item', (el) => el.classList.remove('checked')); + this.elComboValue.value = ''; + this.onChange(); return; } const scope = elItem.getAttribute('data-scope'); if (scope) { // scoped items could only be checked one at a time - const elSelected = elDropdown.querySelector(`.menu > .item.checked[data-scope="${CSS.escape(scope)}"]`); + const elSelected = this.elDropdown.querySelector(`.menu > .item.checked[data-scope="${CSS.escape(scope)}"]`); if (elSelected === elItem) { elItem.classList.toggle('checked'); } else { - queryElems(elDropdown, `.menu > .item[data-scope="${CSS.escape(scope)}"]`, (el) => el.classList.remove('checked')); + queryElems(this.elDropdown, `.menu > .item[data-scope="${CSS.escape(scope)}"]`, (el) => el.classList.remove('checked')); elItem.classList.toggle('checked', true); } } else { - 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; + if (this.selectionMode === 'multiple') { + elItem.classList.toggle('checked'); + } else { + queryElems(this.elDropdown, `.menu > .item.checked`, (el) => el.classList.remove('checked')); + elItem.classList.toggle('checked', true); } } - for (const value of changedValues) { - if (!initialValues.includes(value)) { - await POST(updateUrl, {data: new URLSearchParams({action: 'attach', id: value})}); - changed = true; + this.elComboValue.value = this.collectCheckedValues().join(','); + this.onChange(); + } + + async onHide() { + if (this.selectionMode === 'multiple') this.doUpdate(); + } + + init() { + // init the checked items from initial value + if (this.elComboValue.value && this.elComboValue.value !== '0' && !queryElems(this.elDropdown, `.menu > .item.checked`).length) { + const values = this.elComboValue.value.split(','); + for (const value of values) { + const elItem = this.elDropdown.querySelector(`.menu > .item[data-value="${CSS.escape(value)}"]`); + elItem?.classList.add('checked'); } + this.updateUiList(values); } - if (changed) issueSidebarReloadConfirmDraftComment(); - }; + this.initialValues = this.collectCheckedValues(); - const syncUiList = (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="${CSS.escape(value)}"]`); - const listItem = el.cloneNode(true) as HTMLElement; - queryElems(listItem, '.item-check-mark, .item-secondary-info', (el) => el.remove()); - elList.append(listItem); - } - const hasItems = Boolean(elList.querySelector('.item:not(.empty-list)')); - toggleElem(elEmptyTip, !hasItems); - }; - - fomanticQuery(elDropdown).dropdown('setting', { - action: 'nothing', // do not hide the menu if user presses Enter - fullTextSearch: 'exact', - async onHide() { - // TODO: support "Esc" to cancel the selection. Use partial page loading to avoid losing inputs. - const changedValues = collectCheckedValues(elDropdown); - syncUiList(changedValues); - if (updateUrl) await updateToBackend(changedValues); - initialValues = changedValues; - }, - }); + this.elDropdown.addEventListener('click', (e) => this.onItemClick(e)); + + fomanticQuery(this.elDropdown).dropdown('setting', { + action: 'nothing', // do not hide the menu if user presses Enter + fullTextSearch: 'exact', + onHide: () => this.onHide(), + }); + } +} + +export function initIssueSidebarComboList(container: HTMLElement) { + new IssueSidebarComboList(container).init(); } diff --git a/web_src/js/features/repo-issue-sidebar.md b/web_src/js/features/repo-issue-sidebar.md index 3022b52d05..6de013f1c2 100644 --- a/web_src/js/features/repo-issue-sidebar.md +++ b/web_src/js/features/repo-issue-sidebar.md @@ -1,7 +1,7 @@ A sidebar combo (dropdown+list) is like this: ```html -
+
@@ -25,3 +25,7 @@ If there is `data-update-url`, it also calls backend to attach/detach the change Also, the changed items will be syncronized to the `ui list` items. The items with the same data-scope only allow one selected at a time. + +The dropdown selection could work in 2 modes: +* single: only one item could be selected, it updates immediately when the item is selected. +* multiple: multiple items could be selected, it defers the update until the dropdown is hidden. diff --git a/web_src/js/features/repo-issue-sidebar.ts b/web_src/js/features/repo-issue-sidebar.ts index 52878848e8..45cd38d533 100644 --- a/web_src/js/features/repo-issue-sidebar.ts +++ b/web_src/js/features/repo-issue-sidebar.ts @@ -1,10 +1,7 @@ import $ from 'jquery'; import {POST} from '../modules/fetch.ts'; -import {updateIssuesMeta} from './repo-common.ts'; -import {svg} from '../svg.ts'; -import {htmlEscape} from 'escape-goat'; import {queryElems, toggleElem} from '../utils/dom.ts'; -import {initIssueSidebarComboList, issueSidebarReloadConfirmDraftComment} from './repo-issue-sidebar-combolist.ts'; +import {initIssueSidebarComboList} from './repo-issue-sidebar-combolist.ts'; function initBranchSelector() { const elSelectBranch = document.querySelector('.ui.dropdown.select-branch'); @@ -34,212 +31,6 @@ function 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) { - issueSidebarReloadConfirmDraftComment(); - } - } - }, - }); - - $listMenu.find('.item:not(.no-select)').on('click', function (e) { - e.preventDefault(); - if (this.classList.contains('ban-change')) { - return false; - } - - hasUpdateAction = $listMenu.data('action') === 'update'; // Update the var - - const clickedItem = this; // eslint-disable-line unicorn/no-this-assignment - const scope = this.getAttribute('data-scope'); - - $(this).parent().find('.item').each(function () { - if (scope) { - // Enable only clicked item for scoped labels - if (this.getAttribute('data-scope') !== scope) { - return; - } - if (this !== clickedItem && !this.classList.contains('checked')) { - return; - } - } else if (this !== clickedItem) { - // Toggle for other labels - return; - } - - if (this.classList.contains('checked')) { - $(this).removeClass('checked'); - $(this).find('.octicon-check').addClass('tw-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('tw-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-assignees-modify') { - return false; - } - - const listIds = []; - $(this).parent().find('.item').each(function () { - if (this.classList.contains('checked')) { - listIds.push($(this).data('id')); - $($(this).data('id-selector')).removeClass('tw-hidden'); - } else { - $($(this).data('id-selector')).addClass('tw-hidden'); - } - }); - if (!listIds.length) { - $noSelect.removeClass('tw-hidden'); - } else { - $noSelect.addClass('tw-hidden'); - } - $($(this).parent().data('id')).val(listIds.join(',')); - return false; - }); - $listMenu.find('.no-select.item').on('click', function (e) { - e.preventDefault(); - if (hasUpdateAction) { - (async () => { - await updateIssuesMeta( - $listMenu.data('update-url'), - 'clear', - $listMenu.data('issue-id'), - '', - ); - issueSidebarReloadConfirmDraftComment(); - })(); - } - - $(this).parent().find('.item').each(function () { - $(this).removeClass('checked'); - $(this).find('.octicon-check').addClass('tw-invisible'); - }); - - if (selector === 'select-assignees-modify') { - return false; - } - - $list.find('.item').each(function () { - $(this).addClass('tw-hidden'); - }); - $noSelect.removeClass('tw-hidden'); - $($(this).parent().data('id')).val(''); - }); -} - -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) { - (async () => { - await updateIssuesMeta( - $menu.data('update-url'), - '', - $menu.data('issue-id'), - $(this).data('id'), - ); - issueSidebarReloadConfirmDraftComment(); - })(); - } - - let icon = ''; - if (input_id === '#milestone_id') { - icon = svg('octicon-milestone', 18, 'tw-mr-2'); - } else if (input_id === '#project_id') { - icon = svg('octicon-project', 18, 'tw-mr-2'); - } else if (input_id === '#assignee_id') { - icon = `avatar`; - } - - $list.find('.selected').html(` - - ${icon} - ${htmlEscape(this.textContent)} - - `); - - $(`.ui${select_id}.list .no-select`).addClass('tw-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) { - (async () => { - await updateIssuesMeta( - $menu.data('update-url'), - '', - $menu.data('issue-id'), - $(this).data('id'), - ); - issueSidebarReloadConfirmDraftComment(); - })(); - } - - $list.find('.selected').html(''); - $list.find('.no-select').removeClass('tw-hidden'); - $(input_id).val(''); - }); -} - function initRepoIssueDue() { const form = document.querySelector('.issue-due-form'); if (!form) return; @@ -257,14 +48,6 @@ export function initRepoIssueSidebar() { initBranchSelector(); initRepoIssueDue(); - // TODO: refactor the legacy initListSubmits&selectItem to initIssueSidebarComboList - initListSubmits('select-assignees', 'assignees'); - initListSubmits('select-assignees-modify', 'assignees'); - selectItem('.select-assignee', '#assignee_id'); - - selectItem('.select-project', '#project_id'); - selectItem('.select-milestone', '#milestone_id'); - // init the combo list: a dropdown for selecting items, and a list for showing selected items and related actions queryElems(document, '.issue-sidebar-combo', (el) => initIssueSidebarComboList(el)); } -- 2.39.5