From 1bff02de55331e11de3627d5c5628feb2cd97387 Mon Sep 17 00:00:00 2001 From: kolaente Date: Tue, 17 Jul 2018 23:23:58 +0200 Subject: [PATCH] Added dependencies for issues (#2196) (#2531) --- custom/conf/app.ini.sample | 3 + .../doc/advanced/config-cheat-sheet.en-us.md | 1 + models/action.go | 4 + models/error.go | 85 +++++++++++ models/issue.go | 44 ++++++ models/issue_comment.go | 137 ++++++++++++------ models/issue_dependency.go | 137 ++++++++++++++++++ models/issue_dependency_test.go | 57 ++++++++ models/migrations/migrations.go | 2 + models/migrations/v70.go | 100 +++++++++++++ models/models.go | 1 + models/repo.go | 6 +- models/repo_unit.go | 2 +- modules/auth/repo_form.go | 1 + modules/context/repo.go | 5 + modules/setting/setting.go | 2 + options/locale/locale_en-US.ini | 28 ++++ public/js/index.js | 39 +++++ routers/api/v1/repo/issue.go | 9 ++ routers/api/v1/repo/pull.go | 5 + routers/repo/issue.go | 33 +++++ routers/repo/issue_dependency.go | 119 +++++++++++++++ routers/repo/pull.go | 12 ++ routers/repo/setting.go | 1 + routers/routes/routes.go | 4 + templates/admin/config.tmpl | 2 + .../repo/issue/view_content/comments.tmpl | 33 ++++- .../repo/issue/view_content/sidebar.tmpl | 137 ++++++++++++++++++ templates/repo/settings/options.tmpl | 6 + 29 files changed, 967 insertions(+), 48 deletions(-) create mode 100644 models/issue_dependency.go create mode 100644 models/issue_dependency_test.go create mode 100644 models/migrations/v70.go create mode 100644 routers/repo/issue_dependency.go diff --git a/custom/conf/app.ini.sample b/custom/conf/app.ini.sample index c4dd741cac..f087e9913b 100644 --- a/custom/conf/app.ini.sample +++ b/custom/conf/app.ini.sample @@ -316,6 +316,9 @@ DEFAULT_KEEP_EMAIL_PRIVATE = false ; Default value for AllowCreateOrganization ; Every new user will have rights set to create organizations depending on this setting DEFAULT_ALLOW_CREATE_ORGANIZATION = true +; Default value for EnableDependencies +; Repositories will use depencies by default depending on this setting +DEFAULT_ENABLE_DEPENDENCIES = true ; Enable Timetracking ENABLE_TIMETRACKING = true ; Default value for EnableTimetracking diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index c7e80b9579..5ed2ce43e7 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -182,6 +182,7 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`. - `CAPTCHA_TYPE`: **image**: \[image, recaptcha\] - `RECAPTCHA_SECRET`: **""**: Go to https://www.google.com/recaptcha/admin to get a secret for recaptcha - `RECAPTCHA_SITEKEY`: **""**: Go to https://www.google.com/recaptcha/admin to get a sitekey for recaptcha +- `DEFAULT_ENABLE_DEPENDENCIES`: **true** Enable this to have dependencies enabled by default. ## Webhook (`webhook`) diff --git a/models/action.go b/models/action.go index a6fb209a41..adf30bb88b 100644 --- a/models/action.go +++ b/models/action.go @@ -477,6 +477,10 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit) err } if err = issue.ChangeStatus(doer, repo, true); err != nil { + // Don't return an error when dependencies are open as this would let the push fail + if IsErrDependenciesLeft(err) { + return nil + } return err } } diff --git a/models/error.go b/models/error.go index 0757044475..029c33aba4 100644 --- a/models/error.go +++ b/models/error.go @@ -1259,3 +1259,88 @@ func IsErrU2FRegistrationNotExist(err error) bool { _, ok := err.(ErrU2FRegistrationNotExist) return ok } + +// .___ ________ .___ .__ +// | | ______ ________ __ ____ \______ \ ____ ______ ____ ____ __| _/____ ____ ____ |__| ____ ______ +// | |/ ___// ___/ | \_/ __ \ | | \_/ __ \\____ \_/ __ \ / \ / __ |/ __ \ / \_/ ___\| |/ __ \ / ___/ +// | |\___ \ \___ \| | /\ ___/ | ` \ ___/| |_> > ___/| | \/ /_/ \ ___/| | \ \___| \ ___/ \___ \ +// |___/____ >____ >____/ \___ >_______ /\___ > __/ \___ >___| /\____ |\___ >___| /\___ >__|\___ >____ > +// \/ \/ \/ \/ \/|__| \/ \/ \/ \/ \/ \/ \/ \/ + +// ErrDependencyExists represents a "DependencyAlreadyExists" kind of error. +type ErrDependencyExists struct { + IssueID int64 + DependencyID int64 +} + +// IsErrDependencyExists checks if an error is a ErrDependencyExists. +func IsErrDependencyExists(err error) bool { + _, ok := err.(ErrDependencyExists) + return ok +} + +func (err ErrDependencyExists) Error() string { + return fmt.Sprintf("issue dependency does already exist [issue id: %d, dependency id: %d]", err.IssueID, err.DependencyID) +} + +// ErrDependencyNotExists represents a "DependencyAlreadyExists" kind of error. +type ErrDependencyNotExists struct { + IssueID int64 + DependencyID int64 +} + +// IsErrDependencyNotExists checks if an error is a ErrDependencyExists. +func IsErrDependencyNotExists(err error) bool { + _, ok := err.(ErrDependencyNotExists) + return ok +} + +func (err ErrDependencyNotExists) Error() string { + return fmt.Sprintf("issue dependency does not exist [issue id: %d, dependency id: %d]", err.IssueID, err.DependencyID) +} + +// ErrCircularDependency represents a "DependencyCircular" kind of error. +type ErrCircularDependency struct { + IssueID int64 + DependencyID int64 +} + +// IsErrCircularDependency checks if an error is a ErrCircularDependency. +func IsErrCircularDependency(err error) bool { + _, ok := err.(ErrCircularDependency) + return ok +} + +func (err ErrCircularDependency) Error() string { + return fmt.Sprintf("circular dependencies exists (two issues blocking each other) [issue id: %d, dependency id: %d]", err.IssueID, err.DependencyID) +} + +// ErrDependenciesLeft represents an error where the issue you're trying to close still has dependencies left. +type ErrDependenciesLeft struct { + IssueID int64 +} + +// IsErrDependenciesLeft checks if an error is a ErrDependenciesLeft. +func IsErrDependenciesLeft(err error) bool { + _, ok := err.(ErrDependenciesLeft) + return ok +} + +func (err ErrDependenciesLeft) Error() string { + return fmt.Sprintf("issue has open dependencies [issue id: %d]", err.IssueID) +} + +// ErrUnknownDependencyType represents an error where an unknown dependency type was passed +type ErrUnknownDependencyType struct { + Type DependencyType +} + +// IsErrUnknownDependencyType checks if an error is ErrUnknownDependencyType +func IsErrUnknownDependencyType(err error) bool { + _, ok := err.(ErrUnknownDependencyType) + return ok +} + +func (err ErrUnknownDependencyType) Error() string { + return fmt.Sprintf("unknown dependency type [type: %d]", err.Type) +} diff --git a/models/issue.go b/models/issue.go index d97266b4ed..c89ffa7d0f 100644 --- a/models/issue.go +++ b/models/issue.go @@ -649,6 +649,20 @@ func (issue *Issue) changeStatus(e *xorm.Session, doer *User, repo *Repository, if issue.IsClosed == isClosed { return nil } + + // Check for open dependencies + if isClosed && issue.Repo.IsDependenciesEnabled() { + // only check if dependencies are enabled and we're about to close an issue, otherwise reopening an issue would fail when there are unsatisfied dependencies + noDeps, err := IssueNoDependenciesLeft(issue) + if err != nil { + return err + } + + if !noDeps { + return ErrDependenciesLeft{issue.ID} + } + } + issue.IsClosed = isClosed if isClosed { issue.ClosedUnix = util.TimeStampNow() @@ -1598,3 +1612,33 @@ func UpdateIssueDeadline(issue *Issue, deadlineUnix util.TimeStamp, doer *User) return sess.Commit() } + +// Get Blocked By Dependencies, aka all issues this issue is blocked by. +func (issue *Issue) getBlockedByDependencies(e Engine) (issueDeps []*Issue, err error) { + return issueDeps, e. + Table("issue_dependency"). + Select("issue.*"). + Join("INNER", "issue", "issue.id = issue_dependency.dependency_id"). + Where("issue_id = ?", issue.ID). + Find(&issueDeps) +} + +// Get Blocking Dependencies, aka all issues this issue blocks. +func (issue *Issue) getBlockingDependencies(e Engine) (issueDeps []*Issue, err error) { + return issueDeps, e. + Table("issue_dependency"). + Select("issue.*"). + Join("INNER", "issue", "issue.id = issue_dependency.issue_id"). + Where("dependency_id = ?", issue.ID). + Find(&issueDeps) +} + +// BlockedByDependencies finds all Dependencies an issue is blocked by +func (issue *Issue) BlockedByDependencies() ([]*Issue, error) { + return issue.getBlockedByDependencies(x) +} + +// BlockingDependencies returns all blocking dependencies, aka all other issues a given issue blocks +func (issue *Issue) BlockingDependencies() ([]*Issue, error) { + return issue.getBlockingDependencies(x) +} diff --git a/models/issue_comment.go b/models/issue_comment.go index 1c7c57dd06..ad276e61f9 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -66,6 +66,10 @@ const ( CommentTypeModifiedDeadline // Removed a due date CommentTypeRemovedDeadline + // Dependency added + CommentTypeAddDependency + //Dependency removed + CommentTypeRemoveDependency ) // CommentTag defines comment tag type @@ -81,23 +85,25 @@ const ( // Comment represents a comment in commit and issue page. type Comment struct { - ID int64 `xorm:"pk autoincr"` - Type CommentType - PosterID int64 `xorm:"INDEX"` - Poster *User `xorm:"-"` - IssueID int64 `xorm:"INDEX"` - Issue *Issue `xorm:"-"` - LabelID int64 - Label *Label `xorm:"-"` - OldMilestoneID int64 - MilestoneID int64 - OldMilestone *Milestone `xorm:"-"` - Milestone *Milestone `xorm:"-"` - AssigneeID int64 - RemovedAssignee bool - Assignee *User `xorm:"-"` - OldTitle string - NewTitle string + ID int64 `xorm:"pk autoincr"` + Type CommentType + PosterID int64 `xorm:"INDEX"` + Poster *User `xorm:"-"` + IssueID int64 `xorm:"INDEX"` + Issue *Issue `xorm:"-"` + LabelID int64 + Label *Label `xorm:"-"` + OldMilestoneID int64 + MilestoneID int64 + OldMilestone *Milestone `xorm:"-"` + Milestone *Milestone `xorm:"-"` + AssigneeID int64 + RemovedAssignee bool + Assignee *User `xorm:"-"` + OldTitle string + NewTitle string + DependentIssueID int64 + DependentIssue *Issue `xorm:"-"` CommitID int64 Line int64 @@ -281,6 +287,15 @@ func (c *Comment) LoadAssigneeUser() error { return nil } +// LoadDepIssueDetails loads Dependent Issue Details +func (c *Comment) LoadDepIssueDetails() (err error) { + if c.DependentIssueID <= 0 || c.DependentIssue != nil { + return nil + } + c.DependentIssue, err = getIssueByID(x, c.DependentIssueID) + return err +} + // MailParticipants sends new comment emails to repository watchers // and mentioned people. func (c *Comment) MailParticipants(e Engine, opType ActionType, issue *Issue) (err error) { @@ -332,22 +347,24 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err if opts.Label != nil { LabelID = opts.Label.ID } + comment := &Comment{ - Type: opts.Type, - PosterID: opts.Doer.ID, - Poster: opts.Doer, - IssueID: opts.Issue.ID, - LabelID: LabelID, - OldMilestoneID: opts.OldMilestoneID, - MilestoneID: opts.MilestoneID, - RemovedAssignee: opts.RemovedAssignee, - AssigneeID: opts.AssigneeID, - CommitID: opts.CommitID, - CommitSHA: opts.CommitSHA, - Line: opts.LineNum, - Content: opts.Content, - OldTitle: opts.OldTitle, - NewTitle: opts.NewTitle, + Type: opts.Type, + PosterID: opts.Doer.ID, + Poster: opts.Doer, + IssueID: opts.Issue.ID, + LabelID: LabelID, + OldMilestoneID: opts.OldMilestoneID, + MilestoneID: opts.MilestoneID, + RemovedAssignee: opts.RemovedAssignee, + AssigneeID: opts.AssigneeID, + CommitID: opts.CommitID, + CommitSHA: opts.CommitSHA, + Line: opts.LineNum, + Content: opts.Content, + OldTitle: opts.OldTitle, + NewTitle: opts.NewTitle, + DependentIssueID: opts.DependentIssueID, } if _, err = e.Insert(comment); err != nil { return nil, err @@ -549,6 +566,39 @@ func createDeleteBranchComment(e *xorm.Session, doer *User, repo *Repository, is }) } +// Creates issue dependency comment +func createIssueDependencyComment(e *xorm.Session, doer *User, issue *Issue, dependentIssue *Issue, add bool) (err error) { + cType := CommentTypeAddDependency + if !add { + cType = CommentTypeRemoveDependency + } + + // Make two comments, one in each issue + _, err = createComment(e, &CreateCommentOptions{ + Type: cType, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + DependentIssueID: dependentIssue.ID, + }) + if err != nil { + return + } + + _, err = createComment(e, &CreateCommentOptions{ + Type: cType, + Doer: doer, + Repo: issue.Repo, + Issue: dependentIssue, + DependentIssueID: issue.ID, + }) + if err != nil { + return + } + + return +} + // CreateCommentOptions defines options for creating comment type CreateCommentOptions struct { Type CommentType @@ -557,17 +607,18 @@ type CreateCommentOptions struct { Issue *Issue Label *Label - OldMilestoneID int64 - MilestoneID int64 - AssigneeID int64 - RemovedAssignee bool - OldTitle string - NewTitle string - CommitID int64 - CommitSHA string - LineNum int64 - Content string - Attachments []string // UUIDs of attachments + DependentIssueID int64 + OldMilestoneID int64 + MilestoneID int64 + AssigneeID int64 + RemovedAssignee bool + OldTitle string + NewTitle string + CommitID int64 + CommitSHA string + LineNum int64 + Content string + Attachments []string // UUIDs of attachments } // CreateComment creates comment of issue or commit. diff --git a/models/issue_dependency.go b/models/issue_dependency.go new file mode 100644 index 0000000000..157e9257c4 --- /dev/null +++ b/models/issue_dependency.go @@ -0,0 +1,137 @@ +// Copyright 2018 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +import ( + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" +) + +// IssueDependency represents an issue dependency +type IssueDependency struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"NOT NULL"` + IssueID int64 `xorm:"UNIQUE(issue_dependency) NOT NULL"` + DependencyID int64 `xorm:"UNIQUE(issue_dependency) NOT NULL"` + CreatedUnix util.TimeStamp `xorm:"created"` + UpdatedUnix util.TimeStamp `xorm:"updated"` +} + +// DependencyType Defines Dependency Type Constants +type DependencyType int + +// Define Dependency Types +const ( + DependencyTypeBlockedBy DependencyType = iota + DependencyTypeBlocking +) + +// CreateIssueDependency creates a new dependency for an issue +func CreateIssueDependency(user *User, issue, dep *Issue) error { + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + + // Check if it aleready exists + exists, err := issueDepExists(sess, issue.ID, dep.ID) + if err != nil { + return err + } + if exists { + return ErrDependencyExists{issue.ID, dep.ID} + } + // And if it would be circular + circular, err := issueDepExists(sess, dep.ID, issue.ID) + if err != nil { + return err + } + if circular { + return ErrCircularDependency{issue.ID, dep.ID} + } + + if _, err := sess.Insert(&IssueDependency{ + UserID: user.ID, + IssueID: issue.ID, + DependencyID: dep.ID, + }); err != nil { + return err + } + + // Add comment referencing the new dependency + if err = createIssueDependencyComment(sess, user, issue, dep, true); err != nil { + return err + } + + return sess.Commit() +} + +// RemoveIssueDependency removes a dependency from an issue +func RemoveIssueDependency(user *User, issue *Issue, dep *Issue, depType DependencyType) (err error) { + sess := x.NewSession() + defer sess.Close() + if err = sess.Begin(); err != nil { + return err + } + + var issueDepToDelete IssueDependency + + switch depType { + case DependencyTypeBlockedBy: + issueDepToDelete = IssueDependency{IssueID: issue.ID, DependencyID: dep.ID} + case DependencyTypeBlocking: + issueDepToDelete = IssueDependency{IssueID: dep.ID, DependencyID: issue.ID} + default: + return ErrUnknownDependencyType{depType} + } + + affected, err := sess.Delete(&issueDepToDelete) + if err != nil { + return err + } + + // If we deleted nothing, the dependency did not exist + if affected <= 0 { + return ErrDependencyNotExists{issue.ID, dep.ID} + } + + // Add comment referencing the removed dependency + if err = createIssueDependencyComment(sess, user, issue, dep, false); err != nil { + return err + } + return sess.Commit() +} + +// Check if the dependency already exists +func issueDepExists(e Engine, issueID int64, depID int64) (bool, error) { + return e.Where("(issue_id = ? AND dependency_id = ?)", issueID, depID).Exist(&IssueDependency{}) +} + +// IssueNoDependenciesLeft checks if issue can be closed +func IssueNoDependenciesLeft(issue *Issue) (bool, error) { + + exists, err := x. + Table("issue_dependency"). + Select("issue.*"). + Join("INNER", "issue", "issue.id = issue_dependency.dependency_id"). + Where("issue_dependency.issue_id = ?", issue.ID). + And("issue.is_closed = ?", "0"). + Exist(&Issue{}) + + return !exists, err +} + +// IsDependenciesEnabled returns if dependecies are enabled and returns the default setting if not set. +func (repo *Repository) IsDependenciesEnabled() bool { + var u *RepoUnit + var err error + if u, err = repo.GetUnit(UnitTypeIssues); err != nil { + log.Trace("%s", err) + return setting.Service.DefaultEnableDependencies + } + return u.IssuesConfig().EnableDependencies +} diff --git a/models/issue_dependency_test.go b/models/issue_dependency_test.go new file mode 100644 index 0000000000..571bce3184 --- /dev/null +++ b/models/issue_dependency_test.go @@ -0,0 +1,57 @@ +// Copyright 2018 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCreateIssueDependency(t *testing.T) { + // Prepare + assert.NoError(t, PrepareTestDatabase()) + + user1, err := GetUserByID(1) + assert.NoError(t, err) + + issue1, err := GetIssueByID(1) + assert.NoError(t, err) + issue2, err := GetIssueByID(2) + assert.NoError(t, err) + + // Create a dependency and check if it was successful + err = CreateIssueDependency(user1, issue1, issue2) + assert.NoError(t, err) + + // Do it again to see if it will check if the dependency already exists + err = CreateIssueDependency(user1, issue1, issue2) + assert.Error(t, err) + assert.True(t, IsErrDependencyExists(err)) + + // Check for circular dependencies + err = CreateIssueDependency(user1, issue2, issue1) + assert.Error(t, err) + assert.True(t, IsErrCircularDependency(err)) + + _ = AssertExistsAndLoadBean(t, &Comment{Type: CommentTypeAddDependency, PosterID: user1.ID, IssueID: issue1.ID}) + + // Check if dependencies left is correct + left, err := IssueNoDependenciesLeft(issue1) + assert.NoError(t, err) + assert.False(t, left) + + // Close #2 and check again + err = issue2.ChangeStatus(user1, issue2.Repo, true) + assert.NoError(t, err) + + left, err = IssueNoDependenciesLeft(issue1) + assert.NoError(t, err) + assert.True(t, left) + + // Test removing the dependency + err = RemoveIssueDependency(user1, issue1, issue2, DependencyTypeBlockedBy) + assert.NoError(t, err) +} diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index cc262d8102..48c4228fe1 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -192,6 +192,8 @@ var migrations = []Migration{ NewMigration("Reformat and remove incorrect topics", reformatAndRemoveIncorrectTopics), // v69 -> v70 NewMigration("move team units to team_unit table", moveTeamUnitsToTeamUnitTable), + // v70 -> v71 + NewMigration("add issue_dependencies", addIssueDependencies), } // Migrate database to current version diff --git a/models/migrations/v70.go b/models/migrations/v70.go new file mode 100644 index 0000000000..4ce1d4ee53 --- /dev/null +++ b/models/migrations/v70.go @@ -0,0 +1,100 @@ +// Copyright 2018 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "fmt" + "time" + + "code.gitea.io/gitea/modules/setting" + + "github.com/go-xorm/xorm" +) + +func addIssueDependencies(x *xorm.Engine) (err error) { + + type IssueDependency struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"NOT NULL"` + IssueID int64 `xorm:"NOT NULL"` + DependencyID int64 `xorm:"NOT NULL"` + Created time.Time `xorm:"-"` + CreatedUnix int64 `xorm:"created"` + Updated time.Time `xorm:"-"` + UpdatedUnix int64 `xorm:"updated"` + } + + if err = x.Sync(new(IssueDependency)); err != nil { + return fmt.Errorf("Error creating issue_dependency_table column definition: %v", err) + } + + // Update Comment definition + // This (copied) struct does only contain fields used by xorm as the only use here is to update the database + + // CommentType defines the comment type + type CommentType int + + // TimeStamp defines a timestamp + type TimeStamp int64 + + type Comment struct { + ID int64 `xorm:"pk autoincr"` + Type CommentType + PosterID int64 `xorm:"INDEX"` + IssueID int64 `xorm:"INDEX"` + LabelID int64 + OldMilestoneID int64 + MilestoneID int64 + OldAssigneeID int64 + AssigneeID int64 + OldTitle string + NewTitle string + DependentIssueID int64 + + CommitID int64 + Line int64 + Content string `xorm:"TEXT"` + + CreatedUnix TimeStamp `xorm:"INDEX created"` + UpdatedUnix TimeStamp `xorm:"INDEX updated"` + + // Reference issue in commit message + CommitSHA string `xorm:"VARCHAR(40)"` + } + + if err = x.Sync(new(Comment)); err != nil { + return fmt.Errorf("Error updating issue_comment table column definition: %v", err) + } + + // RepoUnit describes all units of a repository + type RepoUnit struct { + ID int64 + RepoID int64 `xorm:"INDEX(s)"` + Type int `xorm:"INDEX(s)"` + Config map[string]interface{} `xorm:"JSON"` + CreatedUnix int64 `xorm:"INDEX CREATED"` + Created time.Time `xorm:"-"` + } + + //Updating existing issue units + units := make([]*RepoUnit, 0, 100) + err = x.Where("`type` = ?", V16UnitTypeIssues).Find(&units) + if err != nil { + return fmt.Errorf("Query repo units: %v", err) + } + for _, unit := range units { + if unit.Config == nil { + unit.Config = make(map[string]interface{}) + } + if _, ok := unit.Config["EnableDependencies"]; !ok { + unit.Config["EnableDependencies"] = setting.Service.DefaultEnableDependencies + } + if _, err := x.ID(unit.ID).Cols("config").Update(unit); err != nil { + return err + } + } + + return err +} diff --git a/models/models.go b/models/models.go index aaf1370fd4..9477b6950a 100644 --- a/models/models.go +++ b/models/models.go @@ -118,6 +118,7 @@ func init() { new(TrackedTime), new(DeletedBranch), new(RepoIndexerStatus), + new(IssueDependency), new(LFSLock), new(Reaction), new(IssueAssignees), diff --git a/models/repo.go b/models/repo.go index c795deee8d..e87883969d 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1345,7 +1345,11 @@ func createRepository(e *xorm.Session, doer, u *User, repo *Repository) (err err units = append(units, RepoUnit{ RepoID: repo.ID, Type: tp, - Config: &IssuesConfig{EnableTimetracker: setting.Service.DefaultEnableTimetracking, AllowOnlyContributorsToTrackTime: setting.Service.DefaultAllowOnlyContributorsToTrackTime}, + Config: &IssuesConfig{ + EnableTimetracker: setting.Service.DefaultEnableTimetracking, + AllowOnlyContributorsToTrackTime: setting.Service.DefaultAllowOnlyContributorsToTrackTime, + EnableDependencies: setting.Service.DefaultEnableDependencies, + }, }) } else if tp == UnitTypePullRequests { units = append(units, RepoUnit{ diff --git a/models/repo_unit.go b/models/repo_unit.go index 49b62ec9cd..1e1778356a 100644 --- a/models/repo_unit.go +++ b/models/repo_unit.go @@ -73,6 +73,7 @@ func (cfg *ExternalTrackerConfig) ToDB() ([]byte, error) { type IssuesConfig struct { EnableTimetracker bool AllowOnlyContributorsToTrackTime bool + EnableDependencies bool } // FromDB fills up a IssuesConfig from serialized format. @@ -165,7 +166,6 @@ func (r *RepoUnit) IssuesConfig() *IssuesConfig { func (r *RepoUnit) ExternalTrackerConfig() *ExternalTrackerConfig { return r.Config.(*ExternalTrackerConfig) } - func getUnitsByRepoID(e Engine, repoID int64) (units []*RepoUnit, err error) { return units, e.Where("repo_id = ?", repoID).Find(&units) } diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 4ea1ba90ec..eea5859fff 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -113,6 +113,7 @@ type RepoSettingForm struct { PullsAllowSquash bool EnableTimetracker bool AllowOnlyContributorsToTrackTime bool + EnableIssueDependencies bool // Admin settings EnableHealthCheck bool diff --git a/modules/context/repo.go b/modules/context/repo.go index 1fe5482ce1..7239b353ff 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -104,6 +104,11 @@ func (r *Repository) CanUseTimetracker(issue *models.Issue, user *models.User) b r.IsWriter() || issue.IsPoster(user.ID) || isAssigned) } +// CanCreateIssueDependencies returns whether or not a user can create dependencies. +func (r *Repository) CanCreateIssueDependencies(user *models.User) bool { + return r.Repository.IsDependenciesEnabled() && r.IsWriter() +} + // GetCommitsCount returns cached commit count for current view func (r *Repository) GetCommitsCount() (int64, error) { var contextName string diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 410cdfbfb5..b44ba4d11b 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -1180,6 +1180,7 @@ var Service struct { DefaultAllowCreateOrganization bool EnableTimetracking bool DefaultEnableTimetracking bool + DefaultEnableDependencies bool DefaultAllowOnlyContributorsToTrackTime bool NoReplyAddress string @@ -1210,6 +1211,7 @@ func newService() { if Service.EnableTimetracking { Service.DefaultEnableTimetracking = sec.Key("DEFAULT_ENABLE_TIMETRACKING").MustBool(true) } + Service.DefaultEnableDependencies = sec.Key("DEFAULT_ENABLE_DEPENDENCIES").MustBool(true) Service.DefaultAllowOnlyContributorsToTrackTime = sec.Key("DEFAULT_ALLOW_ONLY_CONTRIBUTORS_TO_TRACK_TIME").MustBool(true) Service.NoReplyAddress = sec.Key("NO_REPLY_ADDRESS").MustString("noreply.example.org") diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 4685a86d76..72e79cf90d 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -782,6 +782,33 @@ issues.due_date_modified = "modified the due date to %s from %s %s" issues.due_date_remove = "removed the due date %s %s" issues.due_date_overdue = "Overdue" issues.due_date_invalid = "The due date is invalid or out of range. Please use the format yyyy-mm-dd." +issues.dependency.title = Dependencies +issues.dependency.issue_no_dependencies = This issue currently doesn't have any dependencies. +issues.dependency.pr_no_dependencies = This pull request currently doesn't have any dependencies. +issues.dependency.add = Add a new dependency... +issues.dependency.cancel = Cancel +issues.dependency.remove = Remove +issues.dependency.issue_number = Issuenumber +issues.dependency.added_dependency = `%[2]s added a new dependency %[3]s` +issues.dependency.removed_dependency = `%[2]s removed a dependency %[3]s` +issues.dependency.issue_closing_blockedby = Closing this pull request is blocked by the following issues +issues.dependency.pr_closing_blockedby = Closing this issue is blocked by the following issues +issues.dependency.issue_close_blocks = This issue blocks closing of the following issues +issues.dependency.pr_close_blocks = This pull request blocks closing of the following issues +issues.dependency.issue_close_blocked = You need to close all issues blocking this issue before you can close it! +issues.dependency.pr_close_blocked = You need to close all issues blocking this pull request before you can merge it! +issues.dependency.blocks_short = Blocks +issues.dependency.blocked_by_short = Depends on +issues.dependency.remove_header = Remove Dependency +issues.dependency.issue_remove_text = This will remove the dependency to this issue. Are you sure? You cannot undo this! +issues.dependency.pr_remove_text = This will remove the dependency to this pull request. Are you sure? You cannot undo this! +issues.dependency.setting = Issues & PRs can have dependencies +issues.dependency.add_error_same_issue = You cannot make an issue depend on itself! +issues.dependency.add_error_dep_issue_not_exist = Dependent issue does not exist! +issues.dependency.add_error_dep_not_exist = Dependency does not exist! +issues.dependency.add_error_dep_exists = Dependency already exists! +issues.dependency.add_error_cannot_create_circular = You cannot create a dependency with two issues blocking each other! +issues.dependency.add_error_dep_not_same_repo = Both issues must be in the same repo! pulls.desc = Enable merge requests and code reviews. pulls.new = New Pull Request @@ -1500,6 +1527,7 @@ config.enable_timetracking = Enable Time Tracking config.default_enable_timetracking = Enable Time Tracking by Default config.default_allow_only_contributors_to_track_time = Let Only Contributors Track Time config.no_reply_address = Hidden Email Domain +config.default_enable_dependencies = Enable issue dependencies by default config.webhook_config = Webhook Configuration config.queue_length = Queue Length diff --git a/public/js/index.js b/public/js/index.js index 21a55d0e44..696b63e778 100644 --- a/public/js/index.js +++ b/public/js/index.js @@ -1769,6 +1769,7 @@ $(document).ready(function () { initTopicbar(); initU2FAuth(); initU2FRegister(); + initIssueList(); // Repo clone url. if ($('#repo-clone-url').length > 0) { @@ -2488,3 +2489,41 @@ function updateDeadline(deadlineString) { } }); } + +function deleteDependencyModal(id, type) { + $('.remove-dependency') + .modal({ + closable: false, + duration: 200, + onApprove: function () { + $('#removeDependencyID').val(id); + $('#dependencyType').val(type); + $('#removeDependencyForm').submit(); + } + }).modal('show') + ; +} + +function initIssueList() { + var repolink = $('#repolink').val(); + $('.new-dependency-drop-list') + .dropdown({ + apiSettings: { + url: '/api/v1/repos' + repolink + '/issues?q={query}', + onResponse: function(response) { + var filteredResponse = {'success': true, 'results': []}; + // Parse the response from the api to work with our dropdown + $.each(response, function(index, issue) { + filteredResponse.results.push({ + 'name' : '#' + issue.number + ' ' + issue.title, + 'value' : issue.id + }); + }); + return filteredResponse; + }, + }, + + fullTextSearch: true + }) + ; +} diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 76f58b244e..f8ef0fe3d9 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -7,6 +7,7 @@ package repo import ( "fmt" + "net/http" "strings" "code.gitea.io/gitea/models" @@ -208,6 +209,10 @@ func CreateIssue(ctx *context.APIContext, form api.CreateIssueOption) { if form.Closed { if err := issue.ChangeStatus(ctx.User, ctx.Repo.Repository, true); err != nil { + if models.IsErrDependenciesLeft(err) { + ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies") + return + } ctx.Error(500, "ChangeStatus", err) return } @@ -325,6 +330,10 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) { } if form.State != nil { if err = issue.ChangeStatus(ctx.User, ctx.Repo.Repository, api.StateClosed == api.StateType(*form.State)); err != nil { + if models.IsErrDependenciesLeft(err) { + ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies") + return + } ctx.Error(500, "ChangeStatus", err) return } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 78d96c647f..c346d81e33 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -6,6 +6,7 @@ package repo import ( "fmt" + "net/http" "strings" "code.gitea.io/git" @@ -378,6 +379,10 @@ func EditPullRequest(ctx *context.APIContext, form api.EditPullRequestOption) { } if form.State != nil { if err = issue.ChangeStatus(ctx.User, ctx.Repo.Repository, api.StateClosed == api.StateType(*form.State)); err != nil { + if models.IsErrDependenciesLeft(err) { + ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this pull request because it still has open dependencies") + return + } ctx.Error(500, "ChangeStatus", err) return } diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 6f7dc44595..27c95cd9dc 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -10,6 +10,7 @@ import ( "fmt" "io" "io/ioutil" + "net/http" "strconv" "strings" "time" @@ -302,6 +303,9 @@ func RetrieveRepoMetas(ctx *context.Context, repo *models.Repository) []*models. } ctx.Data["Branches"] = brs + // Contains true if the user can create issue dependencies + ctx.Data["CanCreateIssueDependencies"] = ctx.Repo.CanCreateIssueDependencies(ctx.User) + return labels } @@ -665,6 +669,9 @@ func ViewIssue(ctx *context.Context) { } } + // Check if the user can use the dependencies + ctx.Data["CanCreateIssueDependencies"] = ctx.Repo.CanCreateIssueDependencies(ctx.User) + // Render comments and and fetch participants. participants[0] = issue.Poster for _, comment = range issue.Comments { @@ -721,6 +728,11 @@ func ViewIssue(ctx *context.Context) { ctx.ServerError("LoadAssigneeUser", err) return } + } else if comment.Type == models.CommentTypeRemoveDependency || comment.Type == models.CommentTypeAddDependency { + if err = comment.LoadDepIssueDetails(); err != nil { + ctx.ServerError("LoadDepIssueDetails", err) + return + } } } @@ -774,6 +786,10 @@ func ViewIssue(ctx *context.Context) { ctx.Data["IsPullBranchDeletable"] = canDelete && pull.HeadRepo != nil && git.IsBranchExist(pull.HeadRepo.RepoPath(), pull.HeadBranch) } + // Get Dependencies + ctx.Data["BlockedByDependencies"], err = issue.BlockedByDependencies() + ctx.Data["BlockingDependencies"], err = issue.BlockingDependencies() + ctx.Data["Participants"] = participants ctx.Data["NumParticipants"] = len(participants) ctx.Data["Issue"] = issue @@ -971,6 +987,12 @@ func UpdateIssueStatus(ctx *context.Context) { } for _, issue := range issues { if err := issue.ChangeStatus(ctx.User, issue.Repo, isClosed); err != nil { + if models.IsErrDependenciesLeft(err) { + ctx.JSON(http.StatusPreconditionFailed, map[string]interface{}{ + "error": "cannot close this issue because it still has open dependencies", + }) + return + } ctx.ServerError("ChangeStatus", err) return } @@ -1034,6 +1056,17 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) { } else { if err := issue.ChangeStatus(ctx.User, ctx.Repo.Repository, form.Status == "close"); err != nil { log.Error(4, "ChangeStatus: %v", err) + + if models.IsErrDependenciesLeft(err) { + if issue.IsPull { + ctx.Flash.Error(ctx.Tr("repo.issues.dependency.pr_close_blocked")) + ctx.Redirect(fmt.Sprintf("%s/pulls/%d", ctx.Repo.RepoLink, issue.Index), http.StatusSeeOther) + } else { + ctx.Flash.Error(ctx.Tr("repo.issues.dependency.issue_close_blocked")) + ctx.Redirect(fmt.Sprintf("%s/issues/%d", ctx.Repo.RepoLink, issue.Index), http.StatusSeeOther) + } + return + } } else { log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed) diff --git a/routers/repo/issue_dependency.go b/routers/repo/issue_dependency.go new file mode 100644 index 0000000000..730271126d --- /dev/null +++ b/routers/repo/issue_dependency.go @@ -0,0 +1,119 @@ +// Copyright 2018 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package repo + +import ( + "fmt" + "net/http" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/context" +) + +// AddDependency adds new dependencies +func AddDependency(ctx *context.Context) { + // Check if the Repo is allowed to have dependencies + if !ctx.Repo.CanCreateIssueDependencies(ctx.User) { + ctx.Error(http.StatusForbidden, "CanCreateIssueDependencies") + return + } + + depID := ctx.QueryInt64("newDependency") + + issueIndex := ctx.ParamsInt64("index") + issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, issueIndex) + if err != nil { + ctx.ServerError("GetIssueByIndex", err) + return + } + + // Redirect + defer ctx.Redirect(fmt.Sprintf("%s/issues/%d", ctx.Repo.RepoLink, issueIndex), http.StatusSeeOther) + + // Dependency + dep, err := models.GetIssueByID(depID) + if err != nil { + ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_issue_not_exist")) + return + } + + // Check if both issues are in the same repo + if issue.RepoID != dep.RepoID { + ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_not_same_repo")) + return + } + + // Check if issue and dependency is the same + if dep.Index == issueIndex { + ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_same_issue")) + return + } + + err = models.CreateIssueDependency(ctx.User, issue, dep) + if err != nil { + if models.IsErrDependencyExists(err) { + ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_exists")) + return + } else if models.IsErrCircularDependency(err) { + ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_cannot_create_circular")) + return + } else { + ctx.ServerError("CreateOrUpdateIssueDependency", err) + return + } + } +} + +// RemoveDependency removes the dependency +func RemoveDependency(ctx *context.Context) { + // Check if the Repo is allowed to have dependencies + if !ctx.Repo.CanCreateIssueDependencies(ctx.User) { + ctx.Error(http.StatusForbidden, "CanCreateIssueDependencies") + return + } + + depID := ctx.QueryInt64("removeDependencyID") + + issueIndex := ctx.ParamsInt64("index") + issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, issueIndex) + if err != nil { + ctx.ServerError("GetIssueByIndex", err) + return + } + + // Redirect + ctx.Redirect(fmt.Sprintf("%s/issues/%d", ctx.Repo.RepoLink, issueIndex), http.StatusSeeOther) + + // Dependency Type + depTypeStr := ctx.Req.PostForm.Get("dependencyType") + + var depType models.DependencyType + + switch depTypeStr { + case "blockedBy": + depType = models.DependencyTypeBlockedBy + case "blocking": + depType = models.DependencyTypeBlocking + default: + ctx.Error(http.StatusBadRequest, "GetDependecyType") + return + } + + // Dependency + dep, err := models.GetIssueByID(depID) + if err != nil { + ctx.ServerError("GetIssueByID", err) + return + } + + if err = models.RemoveIssueDependency(ctx.User, issue, dep, depType); err != nil { + if models.IsErrDependencyNotExists(err) { + ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_not_exist")) + return + } + ctx.ServerError("RemoveIssueDependency", err) + return + } +} diff --git a/routers/repo/pull.go b/routers/repo/pull.go index dfe07ca44a..c8d65731ee 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -524,6 +524,18 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) { pr.Issue = issue pr.Issue.Repo = ctx.Repo.Repository + + noDeps, err := models.IssueNoDependenciesLeft(issue) + if err != nil { + return + } + + if !noDeps { + ctx.Flash.Error(ctx.Tr("repo.issues.dependency.pr_close_blocked")) + ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) + return + } + if err = pr.Merge(ctx.User, ctx.Repo.GitRepo, models.MergeStyle(form.Do), message); err != nil { if models.IsErrInvalidMergeStyle(err) { ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option")) diff --git a/routers/repo/setting.go b/routers/repo/setting.go index 0d44cb50a8..fa3bd434d5 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -202,6 +202,7 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) { Config: &models.IssuesConfig{ EnableTimetracker: form.EnableTimetracker, AllowOnlyContributorsToTrackTime: form.AllowOnlyContributorsToTrackTime, + EnableDependencies: form.EnableIssueDependencies, }, }) } diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 84158de21b..3eaaff60b6 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -523,6 +523,10 @@ func RegisterRoutes(m *macaron.Macaron) { m.Post("/title", repo.UpdateIssueTitle) m.Post("/content", repo.UpdateIssueContent) m.Post("/watch", repo.IssueWatch) + m.Group("/dependency", func() { + m.Post("/add", repo.AddDependency) + m.Post("/delete", repo.RemoveDependency) + }) m.Combo("/comments").Post(bindIgnErr(auth.CreateCommentForm{}), repo.NewComment) m.Group("/times", func() { m.Post("/add", bindIgnErr(auth.AddTimeManuallyForm{}), repo.AddTimeManually) diff --git a/templates/admin/config.tmpl b/templates/admin/config.tmpl index 21aa8756fd..c3d76687f1 100644 --- a/templates/admin/config.tmpl +++ b/templates/admin/config.tmpl @@ -150,6 +150,8 @@ {{end}}
{{.i18n.Tr "admin.config.no_reply_address"}}
{{if .Service.NoReplyAddress}}{{.Service.NoReplyAddress}}{{else}}-{{end}}
+
{{.i18n.Tr "admin.config.default_enable_dependencies"}}
+
{{.i18n.Tr "admin.config.active_code_lives"}}
{{.Service.ActiveCodeLives}} {{.i18n.Tr "tool.raw_minutes"}}
diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 4f185e4ae6..6a8cd1dacb 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -1,7 +1,7 @@ {{range .Issue.Comments}} {{ $createdStr:= TimeSinceUnix .CreatedUnix $.Lang }} - + {{if eq .Type 0}}
@@ -65,7 +65,6 @@ {{end}}
- {{else if eq .Type 1}}
@@ -233,5 +232,33 @@ {{$.i18n.Tr "repo.issues.due_date_remove" .Content $createdStr | Safe}}
- {{end}} + {{else if eq .Type 19}} +
+ + + + + + {{$.i18n.Tr "repo.issues.dependency.added_dependency" .Poster.HomeLink .Poster.Name $createdStr | Safe}} + + +
+ {{else if eq .Type 20}} +
+ + + + + + {{$.i18n.Tr "repo.issues.dependency.removed_dependency" .Poster.HomeLink .Poster.Name $createdStr | Safe}} + + +
+ {{end}} {{end}} diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index 11552d08d0..606ac303e6 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -249,5 +249,142 @@ {{end}} + + {{if .Repository.IsDependenciesEnabled}} +
+ +
+ {{.i18n.Tr "repo.issues.dependency.title"}} +
+ {{if .BlockedByDependencies}} + + {{.i18n.Tr "repo.issues.dependency.blocked_by_short"}}: + +
+ {{range .BlockedByDependencies}} +
+
+ {{if $.CanCreateIssueDependencies}} + + + + {{end}} + {{if .IsClosed}} +
+ +
+ {{else}} +
+ +
+ {{end}} +
+
#{{.Index}}
+ {{.Title}} +
+ {{end}} +
+ {{end}} + + {{if .BlockingDependencies}} + + {{.i18n.Tr "repo.issues.dependency.blocks_short"}}: + +
+ {{range .BlockingDependencies}} +
+
+ {{if $.CanCreateIssueDependencies}} + + + + {{end}} + {{if .IsClosed}} +
+ +
+ {{else}} +
+ +
+ {{end}} +
+
#{{.Index}}
+ {{.Title}} +
+ {{end}} +
+ {{end}} + + {{if (and (not .BlockedByDependencies) (not .BlockingDependencies))}} +

{{if .Issue.IsPull}} + {{.i18n.Tr "repo.issues.dependency.pr_no_dependencies"}} + {{else}} + {{.i18n.Tr "repo.issues.dependency.issue_no_dependencies"}} + {{end}}

+ {{end}} + + {{if .CanCreateIssueDependencies}} +
+
+ {{$.CsrfTokenHtml}} +
+ + +
+
+
+ {{end}} +
+{{if .CanCreateIssueDependencies}} + + + + + +{{end}} + {{end}} diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index aa6a76dbf4..744fa82062 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -153,6 +153,12 @@ {{end}} +
+
+ + +
+