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

Show depending and blocked PRs in the PR list #29117

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
aa061b4
enhancement: Show blocked_by and blocking in issue-list-view
zokkis Feb 9, 2024
925ac87
refactor: extract to subtemplate
zokkis Feb 9, 2024
2b39981
chore: resolve lint errors
zokkis Feb 9, 2024
ad4d893
refactor: use translations to support rtl
zokkis Feb 9, 2024
5c35d88
enhancement: shows divider
zokkis Feb 9, 2024
67797f5
chore: lowercase
zokkis Feb 9, 2024
586a42b
chore: lint -> double-quotes
zokkis Feb 9, 2024
5b8d4aa
enhancement: line-through for closed issues
zokkis Feb 10, 2024
20dcbad
review: Dependencies as template.HTML
zokkis Feb 10, 2024
f9726ea
style: middot as divider
zokkis Feb 11, 2024
d460a5c
performance: better sql
zokkis Feb 13, 2024
d6acba1
style: better popups
zokkis Feb 13, 2024
28bca98
style: better selector for middot
zokkis Feb 13, 2024
eff4d30
chore: lint fix
zokkis Feb 13, 2024
f2ae3bd
refactor: use xorm builder
zokkis Feb 15, 2024
0df1b37
enhancement: show dependencies only with enabled setting
zokkis Feb 17, 2024
c68b79f
chore: update swagger
zokkis Feb 17, 2024
c0d5bc4
chore: add documentation
zokkis Feb 21, 2024
f41b8fb
chore: corrected 'retrieve'
zokkis Feb 21, 2024
58d7e4f
Apply suggestions from code review
6543 Feb 22, 2024
e0a4164
render issue-link in go-template
zokkis Feb 22, 2024
9e33175
add final new line
zokkis Feb 23, 2024
c2d67ff
added test
zokkis Feb 25, 2024
fa199e7
move issues to be tested into higher range
6543 Feb 26, 2024
2994945
use tailwind
zokkis Mar 4, 2024
86b1006
remove option from api
zokkis Mar 4, 2024
899f5ad
chore: typo fix
zokkis Mar 4, 2024
40c3343
remove join
zokkis Mar 4, 2024
611cccb
added index to IssueDependency
zokkis Mar 4, 2024
44cd718
Fix swagger
lunny Mar 8, 2024
33fb710
Merge branch 'main' into enhancement/show-blocking-in-issue-list
silverwind Mar 9, 2024
3051b0c
Merge remote-tracking branch 'origin/main' into enhancement/show-bloc…
zokkis Apr 1, 2024
c897fe6
improv: removed unused
zokkis Apr 1, 2024
6ed8925
add svgs
silverwind Apr 4, 2024
f407475
Update templates/shared/issue_link.tmpl
silverwind Apr 14, 2024
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
17 changes: 17 additions & 0 deletions models/fixtures/issue_dependency.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
-
id: 1
user_id: 40
issue_id: 21
dependency_id: 20

-
id: 2
user_id: 40
issue_id: 21
dependency_id: 22

-
id: 3
user_id: 40
issue_id: 20
dependency_id: 22
4 changes: 2 additions & 2 deletions models/issues/dependency.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ func (err ErrUnknownDependencyType) Unwrap() error {
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"`
IssueID int64 `xorm:"UNIQUE(issue_dependency) NOT NULL index"`
DependencyID int64 `xorm:"UNIQUE(issue_dependency) NOT NULL index"`
Comment on lines +110 to +111
Copy link
Contributor

Choose a reason for hiding this comment

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

Two nits:

  1. Does it need a migration? For long time I have the question:
    • If a migration is a must, it should be documented in guideline and explained.
    • If a migration is not a must, then we do not need to require to write migrations
  2. The index for IssueID seems redundant, because issue_dependency should already provide the index for it.

CreatedUnix timeutil.TimeStamp `xorm:"created"`
UpdatedUnix timeutil.TimeStamp `xorm:"updated"`
}
Expand Down
9 changes: 7 additions & 2 deletions models/issues/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -580,9 +580,14 @@ func IsUserParticipantsOfIssue(ctx context.Context, user *user_model.User, issue
}

// DependencyInfo represents high level information about an issue which is a dependency of another issue.
// this type is used in func `BlockingDependenciesMap` and `BlockedByDependenciesMap` as xorm intermediate type to retrieve info from joined tables
type DependencyInfo struct {
6543 marked this conversation as resolved.
Show resolved Hide resolved
Issue `xorm:"extends"`
repo_model.Repository `xorm:"extends"`
Issue `xorm:"extends"` // an issue/pull that depend on issue_id or is blocked by issue_id. the exact usage is determined by the function using this type
repo_model.Repository `xorm:"extends"` // the repo, that owns Issue

// fields from `IssueDependency`
IssueID int64 `xorm:"NOT NULL"` // id of the issue/pull the that is used for the selection of dependent issues
DependencyID int64 `xorm:"NOT NULL"` // id of the issue/pull the that is used for the selection of blocked issues
}

// GetParticipantIDsByIssue returns all userIDs who are participated in comments of an issue and issue author
Expand Down
52 changes: 52 additions & 0 deletions models/issues/issue_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -631,3 +631,55 @@ func (issues IssueList) LoadIsRead(ctx context.Context, userID int64) error {

return nil
}

func (issues IssueList) BlockingDependenciesMap(ctx context.Context) (issueDepsMap map[int64][]*DependencyInfo, err error) {
var issueDeps []*DependencyInfo

err = db.GetEngine(ctx).
Table("issue").
Join("INNER", "repository", "repository.id = issue.repo_id").
6543 marked this conversation as resolved.
Show resolved Hide resolved
Join("INNER", "issue_dependency", "issue_dependency.issue_id = issue.id").
Where(builder.In("issue_dependency.dependency_id", issues.getIssueIDs())).
zokkis marked this conversation as resolved.
Show resolved Hide resolved
// sort by repo id then index
Asc("`issue`.`repo_id`").
Asc("`issue`.`index`").
Find(&issueDeps)
if err != nil {
return nil, err
}

issueDepsMap = make(map[int64][]*DependencyInfo, len(issues))
for _, depInfo := range issueDeps {
depInfo.Issue.Repo = &depInfo.Repository

issueDepsMap[depInfo.DependencyID] = append(issueDepsMap[depInfo.DependencyID], depInfo)
}

return issueDepsMap, nil
}

func (issues IssueList) BlockedByDependenciesMap(ctx context.Context) (issueDepsMap map[int64][]*DependencyInfo, err error) {
var issueDeps []*DependencyInfo

err = db.GetEngine(ctx).
Table("issue").
Join("INNER", "repository", "repository.id = issue.repo_id").
Copy link
Member

Choose a reason for hiding this comment

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

The same as above.

Join("INNER", "issue_dependency", "issue_dependency.dependency_id = issue.id").
Where(builder.In("issue_dependency.issue_id", issues.getIssueIDs())).
zokkis marked this conversation as resolved.
Show resolved Hide resolved
// sort by repo id then index
Asc("`issue`.`repo_id`").
Asc("`issue`.`index`").
Find(&issueDeps)
if err != nil {
return nil, err
}

issueDepsMap = make(map[int64][]*DependencyInfo, len(issues))
for _, depInfo := range issueDeps {
depInfo.Issue.Repo = &depInfo.Repository

issueDepsMap[depInfo.IssueID] = append(issueDepsMap[depInfo.IssueID], depInfo)
}

return issueDepsMap, nil
}
136 changes: 136 additions & 0 deletions models/issues/issue_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
package issues_test

import (
"cmp"
"slices"
"testing"

"code.gitea.io/gitea/models/db"
issues_model "code.gitea.io/gitea/models/issues"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/setting"

Expand Down Expand Up @@ -73,3 +76,136 @@ func TestIssueList_LoadAttributes(t *testing.T) {
}
}
}

func TestIssueList_BlockingDependenciesMap(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
issueList := issues_model.IssueList{
unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 20}),
unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 21}),
unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 22}),
}

blockingDependenciesMap, err := issueList.BlockingDependenciesMap(db.DefaultContext)
assert.NoError(t, err)
if assert.Len(t, blockingDependenciesMap, 2) {
var keys []int64
for k := range blockingDependenciesMap {
keys = append(keys, k)
}
slices.Sort(keys)
assert.EqualValues(t, []int64{20, 22}, keys)

if assert.Len(t, blockingDependenciesMap[20], 1) {
expectIssuesDependencyInfo(t,
&issues_model.DependencyInfo{
IssueID: 21,
DependencyID: 20,
Issue: issues_model.Issue{ID: 21},
Repository: repo_model.Repository{ID: 60},
},
blockingDependenciesMap[20][0])
}
if assert.Len(t, blockingDependenciesMap[22], 2) {
list := sortIssuesDependencyInfos(blockingDependenciesMap[22])
expectIssuesDependencyInfo(t, &issues_model.DependencyInfo{
IssueID: 20,
DependencyID: 22,
Issue: issues_model.Issue{ID: 20},
Repository: repo_model.Repository{ID: 23},
}, list[0])
expectIssuesDependencyInfo(t, &issues_model.DependencyInfo{
IssueID: 21,
DependencyID: 22,
Issue: issues_model.Issue{ID: 21},
Repository: repo_model.Repository{ID: 60},
}, list[1])
}
}

issueList = issues_model.IssueList{
unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 21}),
unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 22}),
}

blockingDependenciesMap, err = issueList.BlockingDependenciesMap(db.DefaultContext)
assert.NoError(t, err)
assert.Len(t, blockingDependenciesMap, 1)
assert.Len(t, blockingDependenciesMap[22], 2)
}

func TestIssueList_BlockedByDependenciesMap(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
issueList := issues_model.IssueList{
unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 20}),
unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 21}),
unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 22}),
}

blockedByDependenciesMap, err := issueList.BlockedByDependenciesMap(db.DefaultContext)
assert.NoError(t, err)
if assert.Len(t, blockedByDependenciesMap, 2) {
var keys []int64
for k := range blockedByDependenciesMap {
keys = append(keys, k)
}
slices.Sort(keys)
assert.EqualValues(t, []int64{20, 21}, keys)

if assert.Len(t, blockedByDependenciesMap[20], 1) {
expectIssuesDependencyInfo(t,
&issues_model.DependencyInfo{
IssueID: 20,
DependencyID: 22,
Issue: issues_model.Issue{ID: 22},
Repository: repo_model.Repository{ID: 61},
},
blockedByDependenciesMap[20][0])
}
if assert.Len(t, blockedByDependenciesMap[21], 2) {
list := sortIssuesDependencyInfos(blockedByDependenciesMap[21])
expectIssuesDependencyInfo(t, &issues_model.DependencyInfo{
IssueID: 21,
DependencyID: 20,
Issue: issues_model.Issue{ID: 20},
Repository: repo_model.Repository{ID: 23},
}, list[0])
expectIssuesDependencyInfo(t, &issues_model.DependencyInfo{
IssueID: 21,
DependencyID: 22,
Issue: issues_model.Issue{ID: 22},
Repository: repo_model.Repository{ID: 61},
}, list[1])
}
}

issueList = issues_model.IssueList{
unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 21}),
unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 22}),
}

blockedByDependenciesMap, err = issueList.BlockedByDependenciesMap(db.DefaultContext)
assert.NoError(t, err)
assert.Len(t, blockedByDependenciesMap, 1)
assert.Len(t, blockedByDependenciesMap[21], 2)
}

func expectIssuesDependencyInfo(t *testing.T, expect, got *issues_model.DependencyInfo) {
if expect == nil {
assert.Nil(t, got)
return
}
if !assert.NotNil(t, got) {
return
}
assert.EqualValues(t, expect.DependencyID, got.DependencyID, "DependencyID")
assert.EqualValues(t, expect.IssueID, got.IssueID, "IssueID")
assert.EqualValues(t, expect.Issue.ID, got.Issue.ID, "RelatedIssueID")
assert.EqualValues(t, expect.Repository.ID, got.Repository.ID, "RelatedIssueRepoID")
}

func sortIssuesDependencyInfos(in []*issues_model.DependencyInfo) []*issues_model.DependencyInfo {
slices.SortFunc(in, func(a, b *issues_model.DependencyInfo) int {
return cmp.Compare(a.DependencyID, b.DependencyID)
})
return in
}
1 change: 1 addition & 0 deletions models/repo/repo_unit.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ func (cfg *IssuesConfig) ToDB() ([]byte, error) {
// PullRequestsConfig describes pull requests config
type PullRequestsConfig struct {
IgnoreWhitespaceConflicts bool
ShowDependencies bool
AllowMerge bool
AllowRebase bool
AllowRebaseMerge bool
Expand Down
3 changes: 3 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1696,7 +1696,9 @@ issues.dependency.issue_close_blocked = You need to close all issues blocking th
issues.dependency.issue_batch_close_blocked = "Cannot batch close issues that you choose, because issue #%d still has open dependencies"
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.blocks_following = blocks:
issues.dependency.blocked_by_short = Depends on
issues.dependency.blocked_by_following = depends on:
issues.dependency.remove_header = Remove Dependency
issues.dependency.issue_remove_text = This will remove the dependency from this issue. Continue?
issues.dependency.pr_remove_text = This will remove the dependency from this pull request. Continue?
Expand Down Expand Up @@ -2118,6 +2120,7 @@ settings.enable_timetracker = Enable Time Tracking
settings.allow_only_contributors_to_track_time = Let Only Contributors Track Time
settings.pulls_desc = Enable Repository Pull Requests
settings.pulls.ignore_whitespace = Ignore Whitespace for Conflicts
settings.pulls.show_dependencies = Show <b>depends on</b> and <b>blocks</b> in Pull Requests list
settings.pulls.enable_autodetect_manual_merge = Enable autodetect manual merge (Note: In some special cases, misjudgments can occur)
settings.pulls.allow_rebase_update = Enable updating pull request branch by rebase
settings.pulls.default_delete_branch_after_merge = Delete pull request branch after merge by default
Expand Down
1 change: 1 addition & 0 deletions public/assets/img/svg/gitea-issue-dependency.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions public/assets/img/svg/gitea-issue-dependent.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
19 changes: 19 additions & 0 deletions routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,25 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt
return
}

if unit, err := repo.GetUnit(ctx, unit.TypePullRequests); err == nil {
if config := unit.PullRequestsConfig(); config.ShowDependencies {
blockingDependenciesMap, err := issues.BlockingDependenciesMap(ctx)
if err != nil {
ctx.ServerError("BlockingDependenciesMap", err)
return
}

blockedByDependenciesMap, err := issues.BlockedByDependenciesMap(ctx)
if err != nil {
ctx.ServerError("BlockedByDependenciesMap", err)
return
}

ctx.Data["BlockingDependenciesMap"] = blockingDependenciesMap
ctx.Data["BlockedByDependenciesMap"] = blockedByDependenciesMap
}
}

if ctx.IsSigned {
if err := issues.LoadIsRead(ctx, ctx.Doer.ID); err != nil {
ctx.ServerError("LoadIsRead", err)
Expand Down
1 change: 1 addition & 0 deletions routers/web/repo/setting/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,7 @@ func SettingsPost(ctx *context.Context) {
Type: unit_model.TypePullRequests,
Config: &repo_model.PullRequestsConfig{
IgnoreWhitespaceConflicts: form.PullsIgnoreWhitespace,
ShowDependencies: form.PullsShowDependencies,
AllowMerge: form.PullsAllowMerge,
AllowRebase: form.PullsAllowRebase,
AllowRebaseMerge: form.PullsAllowRebaseMerge,
Expand Down
1 change: 1 addition & 0 deletions services/forms/repo_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ type RepoSettingForm struct {
EnablePulls bool
EnableActions bool
PullsIgnoreWhitespace bool
PullsShowDependencies bool
PullsAllowMerge bool
PullsAllowRebase bool
PullsAllowRebaseMerge bool
Expand Down
6 changes: 6 additions & 0 deletions templates/repo/settings/options.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,12 @@
<label>{{ctx.Locale.Tr "repo.settings.pulls.ignore_whitespace"}}</label>
</div>
</div>
<div class="field">
<div class="ui checkbox">
<input name="pulls_show_dependencies" type="checkbox" {{if and $pullRequestEnabled ($prUnit.PullRequestsConfig.ShowDependencies)}}checked{{end}}>
<label>{{ctx.Locale.Tr "repo.settings.pulls.show_dependencies"}}</label>
</div>
</div>
</div>
{{end}}

Expand Down
9 changes: 9 additions & 0 deletions templates/shared/issue_dependency.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{{if .Dependencies}}
<div class="flex-text-inline">
{{ctx.Locale.Tr .TitleKey}}
{{range $i, $dependency := .Dependencies}}
{{if gt $i 0}}<span class="-tw-ml-1">, </span>{{end}}
{{template "shared/issue_link" $dependency.Issue}}
{{end}}
</div>
{{end}}
1 change: 1 addition & 0 deletions templates/shared/issue_link.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<a href="{{.Link}}" class="{{if .IsClosed}}tw-line-through {{end}}tw-ml-1 ref-issue">#{{.Index}}</a>
10 changes: 10 additions & 0 deletions templates/shared/issuelist.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,16 @@
</span>
</span>
{{end}}
{{if $.BlockedByDependenciesMap}}
{{template "shared/issue_dependency" (dict
"Dependencies" (index $.BlockedByDependenciesMap .ID)
"TitleKey" "repo.issues.dependency.blocked_by_following")}}
{{end}}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{{end}}
{{end}}

{{if $.BlockingDependenciesMap}}
{{template "shared/issue_dependency" (dict
"Dependencies" (index $.BlockingDependenciesMap .ID)
"TitleKey" "repo.issues.dependency.blocks_following")}}
{{end}}
{{if .IsPull}}
{{$approveOfficial := call $approvalCounts .ID "approve"}}
{{$rejectOfficial := call $approvalCounts .ID "reject"}}
Expand Down
1 change: 1 addition & 0 deletions web_src/svg/gitea-issue-dependency.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions web_src/svg/gitea-issue-dependent.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.