Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add close issue on move flag to board and add a cron to remove closed issues from board with that flag #28800

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions models/project/board.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,12 @@ var BoardColorPattern = regexp.MustCompile("^#[0-9a-fA-F]{6}$")

// Board is used to represent boards on a project
type Board struct {
ID int64 `xorm:"pk autoincr"`
Title string
Default bool `xorm:"NOT NULL DEFAULT false"` // issues not assigned to a specific board will be assigned to this board
Sorting int8 `xorm:"NOT NULL DEFAULT 0"`
Color string `xorm:"VARCHAR(7)"`
ID int64 `xorm:"pk autoincr"`
Title string
Default bool `xorm:"NOT NULL DEFAULT false"` // issues not assigned to a specific board will be assigned to this board
Sorting int8 `xorm:"NOT NULL DEFAULT 0"`
Color string `xorm:"VARCHAR(7)"`
CloseIssueOnMove bool `xorm:"DEFAULT false"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as there are new fields added to a model struct, could you add a migration to add this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as there are new fields added to a model struct, could you add a migration to add this?

on a sidenote, regarding migration, i actually broke our prod with a migration(not this pr), it worked after i removed the migration, is there some doc about how to do migration correctly in gitea ?


ProjectID int64 `xorm:"INDEX NOT NULL"`
CreatorID int64 `xorm:"NOT NULL"`
Expand Down
3 changes: 2 additions & 1 deletion models/project/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ type ProjectIssue struct { //revive:disable-line:exported
ProjectBoardID int64 `xorm:"INDEX"`

// the sorting order on the board
Sorting int64 `xorm:"NOT NULL DEFAULT 0"`
Sorting int64 `xorm:"NOT NULL DEFAULT 0"`
CloseIssueOnMove bool `xorm:"DEFAULT false"`
}

func init() {
Expand Down
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -2766,6 +2766,7 @@ dashboard.sync_external_users = Synchronize external user data
dashboard.cleanup_hook_task_table = Cleanup hook_task table
dashboard.cleanup_packages = Cleanup expired packages
dashboard.cleanup_actions = Cleanup actions expired logs and artifacts
dashboard.project_board_actions = Cleanup project closed issues
dashboard.server_uptime = Server Uptime
dashboard.current_goroutine = Current Goroutines
dashboard.current_memory_usage = Current Memory Usage
Expand Down
24 changes: 20 additions & 4 deletions routers/web/org/projects.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"code.gitea.io/gitea/modules/web"
shared_user "code.gitea.io/gitea/routers/web/shared/user"
"code.gitea.io/gitea/services/forms"
issue_service "code.gitea.io/gitea/services/issue"
)

const (
Expand Down Expand Up @@ -542,10 +543,11 @@ func AddBoardToProjectPost(ctx *context.Context) {
}

if err := project_model.NewBoard(ctx, &project_model.Board{
ProjectID: project.ID,
Title: form.Title,
Color: form.Color,
CreatorID: ctx.Doer.ID,
ProjectID: project.ID,
Title: form.Title,
Color: form.Color,
CreatorID: ctx.Doer.ID,
CloseIssueOnMove: form.Close,
}); err != nil {
ctx.ServerError("NewProjectBoard", err)
return
Expand Down Expand Up @@ -747,5 +749,19 @@ func MoveIssues(ctx *context.Context) {
return
}

if board.CloseIssueOnMove {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to remove the if nesting, you could do something like

if !board.CloseIssueOnMove {
ctx.JSONOK()
return
}
//... board close logic

issueID, err := strconv.ParseInt(ctx.FormString("issueID"), 10, 64)
if err != nil {
ctx.ServerError("moved issueID is required", err)
return
}

err = issue_service.CloseIssue(ctx, issueID)
if err != nil {
ctx.ServerError("MoveIssuesOnProjectBoard unable to close issue", err)
return
}
}

ctx.JSONOK()
}
25 changes: 21 additions & 4 deletions routers/web/repo/projects.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"net/http"
"net/url"
"strconv"
"strings"

"code.gitea.io/gitea/models/db"
Expand All @@ -25,6 +26,7 @@ import (
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/services/forms"
issue_service "code.gitea.io/gitea/services/issue"
)

const (
Expand Down Expand Up @@ -483,10 +485,11 @@ func AddBoardToProjectPost(ctx *context.Context) {
}

if err := project_model.NewBoard(ctx, &project_model.Board{
ProjectID: project.ID,
Title: form.Title,
Color: form.Color,
CreatorID: ctx.Doer.ID,
ProjectID: project.ID,
Title: form.Title,
Color: form.Color,
CreatorID: ctx.Doer.ID,
CloseIssueOnMove: form.Close,
}); err != nil {
ctx.ServerError("NewProjectBoard", err)
return
Expand Down Expand Up @@ -696,5 +699,19 @@ func MoveIssues(ctx *context.Context) {
return
}

if board.CloseIssueOnMove {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

issueID, err := strconv.ParseInt(ctx.FormString("issueID"), 10, 64)
if err != nil {
ctx.ServerError("moved issueID is required", err)
return
}

err = issue_service.CloseIssue(ctx, issueID)
if err != nil {
ctx.ServerError("MoveIssuesOnProjectBoard unable to close issue", err)
return
}
}

ctx.JSONOK()
}
119 changes: 119 additions & 0 deletions services/actions/projects.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
// Copyright 2023 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package actions

import (
"context"
"time"

"code.gitea.io/gitea/models/db"
issues_model "code.gitea.io/gitea/models/issues"
project_model "code.gitea.io/gitea/models/project"
"code.gitea.io/gitea/modules/log"
"xorm.io/builder"
)

// Cleanup removes closed issues from project board with close on move flag set to true
func CleanupProjectIssues(taskCtx context.Context, olderThan time.Duration) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while this function would work with smaller instances, the resource consumption with larger ones can grow significantly with the several selects and no limits/pagination.

Could this logic be done with SQL itself, a join on projects where they have closeonmove, with issues that belong to a board that are closed? Then a separate statement to rm the issue from the project? Then in the rm statement(s) you could batch rm the rows from the DB?

log.Info("CRON: remove closed issues from boards with close on move flag set to true starting...")

projects, err := getAllProjects(taskCtx)
if err != nil {
return err
}
for _, project := range projects {
boards, err := getAllProjectBoardWithCloseOnMove(taskCtx, project)
if err != nil {
log.Error("Cannot get boards of project ID %d: %v", project.ID, err)
continue
}
log.Info("Found %d boards with close on move true", len(boards))
for _, board := range boards {
issues, err := getAllIssuesOfBoard(taskCtx, board)
if err != nil {
log.Error("Cannot get issues of board ID %d: %v", board.ID, err)
continue
}
issuesToBeRemoved, err := getAllIssuesToBeRemoved(taskCtx, issues)
if err != nil {
log.Error("Cannot get issues of to be removed of board ID %d: %v", board.ID, err)
continue
}
for _, issueToBeRemoved := range issuesToBeRemoved {
err = removeIssueFromProject(taskCtx, issueToBeRemoved, project)
if err != nil {
log.Error("Cannot remove issue ID %d from board ID %d: %v", issueToBeRemoved.ID, board.ID, err)
continue
}
log.Info("Removed issue ID %d from board ID %d", issueToBeRemoved.ID, board.ID)
}
log.Info("completed removing closed issues from board ID %d", board.ID)
}
log.Info("completed removing closed issues project ID %d", project.ID)
}

log.Info("CRON: remove closed issues from boards with close on move flag true completed.")

return nil
}

func getAllProjects(ctx context.Context) ([]project_model.Project, error) {
var projects []project_model.Project

err := db.GetEngine(ctx).Table("project").Select("*").Find(&projects)
if err != nil {
log.Error("unable to read project db %v", err)
return projects, err
}
return projects, nil
}

func getAllProjectBoardWithCloseOnMove(ctx context.Context, project project_model.Project) ([]project_model.Board, error) {
var boards []project_model.Board

err := db.GetEngine(ctx).Table("project_board").Select("*").Where(builder.Eq{"project_id": project.ID}).Find(&boards)
if err != nil {
log.Error("unable to read project_board db %v", err)
return boards, err
}
return boards, nil
}

func getAllIssuesOfBoard(ctx context.Context, board project_model.Board) ([]int64, error) {
var issueIDs []int64

err := db.GetEngine(ctx).Table("project_issue").Select("issue_id").Where(builder.Eq{"project_id": board.ProjectID, "project_board_id": board.ID}).Find(&issueIDs)
if err != nil {
log.Error("unable to read project_issue db %v", err)
return issueIDs, err
}
return issueIDs, nil
}

func getAllIssuesToBeRemoved(ctx context.Context, issueIDs []int64) ([]issues_model.Issue, error) {
var issues []issues_model.Issue

err := db.GetEngine(ctx).Table("issue").Select("*").Where(builder.Eq{"is_closed": 1}).Where(builder.In("id", issueIDs)).Find(&issues)
if err != nil {
log.Error("unable to read issue db %v", err)
return issues, err
}

return issues, nil
}

func removeIssueFromProject(ctx context.Context, issue issues_model.Issue, project project_model.Project) error {
projectIssue := &project_model.ProjectIssue{
IssueID: issue.ID,
ProjectID: project.ID,
}

_, err := db.GetEngine(ctx).Table("project_issue").Delete(&projectIssue)
if err != nil {
log.Error("unable to delete project_issue db %v", err)
return err
}

return nil
}
15 changes: 15 additions & 0 deletions services/cron/tasks_basic.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,20 @@ func registerActionsCleanup() {
})
}

func registerProjectsIssuesCleanup() {
RegisterTaskFatal("project_board_actions", &OlderThanConfig{
BaseConfig: BaseConfig{
Enabled: true,
RunAtStart: true,
Schedule: "@weekly",
},
OlderThan: 24 * time.Hour,
}, func(ctx context.Context, _ *user_model.User, config Config) error {
realConfig := config.(*OlderThanConfig)
return actions.CleanupProjectIssues(ctx, realConfig.OlderThan)
})
}

func initBasicTasks() {
if setting.Mirror.Enabled {
registerUpdateMirrorTask()
Expand All @@ -190,4 +204,5 @@ func initBasicTasks() {
if setting.Actions.Enabled {
registerActionsCleanup()
}
registerProjectsIssuesCleanup()
}
1 change: 1 addition & 0 deletions services/forms/repo_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,7 @@ type EditProjectBoardForm struct {
Title string `binding:"Required;MaxSize(100)"`
Sorting int8
Color string `binding:"MaxSize(7)"`
Close bool
}

// _____ .__.__ __
Expand Down
30 changes: 30 additions & 0 deletions services/issue/project.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2024 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package issue

import (
"errors"

issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/log"
)

func CloseIssue(ctx *context.Context, issueID int64) error {
issue, err := issues_model.GetIssueByID(ctx, issueID)
if err != nil {
return errors.New("failed getting issue")
}

if err := ChangeStatus(ctx, issue, ctx.Doer, "", true); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could also remove if nesting here by checking if err is nil and moving the status change function call prior to the if, and then return in the if, and after the if, continue with the remaining logic.

log.Error("ChangeStatus: %v", err)

if issues_model.IsErrDependenciesLeft(err) {
ctx.JSONError(ctx.Tr("repo.issues.dependency.issue_close_blocked"))
return err
}
}

return nil
}
9 changes: 9 additions & 0 deletions templates/projects/view.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@
</div>
</div>

<div class="field">
<label for="new_project_column_project_close_on_move">Close Issue on Move</label>
<div>
<input
type="checkbox" name="new_project_column_project_close_on_move" id="new_project_column_project_close_on_move" class="gt-mb-4"
>
</div>
</div>

<div class="text right actions">
<button class="ui cancel button">{{ctx.Locale.Tr "settings.cancel"}}</button>
<button data-url="{{$.Link}}" class="ui primary button" id="new_project_column_submit">{{ctx.Locale.Tr "repo.projects.column.new_submit"}}</button>
Expand Down
9 changes: 5 additions & 4 deletions web_src/js/features/repo-projects.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ function updateIssueCount(cards) {
parent.getElementsByClassName('project-column-issue-count')[0].textContent = cnt;
}

function createNewColumn(url, columnTitle, projectColorInput) {
function createNewColumn(url, columnTitle, projectColorInput, closeOnIssueMove) {
$.ajax({
url,
data: JSON.stringify({title: columnTitle.val(), color: projectColorInput.val()}),
data: JSON.stringify({title: columnTitle.val(), color: projectColorInput.val(), close: closeOnIssueMove.prop('checked')}),
headers: {
'X-Csrf-Token': csrfToken,
},
Expand All @@ -39,7 +39,7 @@ function moveIssue({item, from, to, oldIndex}) {
};

$.ajax({
url: `${to.getAttribute('data-url')}/move`,
url: `${to.getAttribute('data-url')}/move?issueID=${item.getAttribute('data-issue')}`,
data: JSON.stringify(columnSorting),
headers: {
'X-Csrf-Token': csrfToken,
Expand Down Expand Up @@ -187,11 +187,12 @@ export function initRepoProject() {
e.preventDefault();
const columnTitle = $('#new_project_column');
const projectColorInput = $('#new_project_column_color_picker');
const closeOnIssueMove = $('#new_project_column_project_close_on_move');
if (!columnTitle.val()) {
return;
}
const url = $(this).data('url');
createNewColumn(url, columnTitle, projectColorInput);
createNewColumn(url, columnTitle, projectColorInput, closeOnIssueMove);
});
}

Expand Down
Loading