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

Allow cross-repository dependencies on issues #7901

Merged
merged 43 commits into from
Oct 31, 2019
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
86442ad
in progress changes for #7405, added ability to add cross-repo depend…
bhalbright Aug 17, 2019
ce3312a
Merge branch 'master' of https://github.com/go-gitea/gitea
bhalbright Aug 17, 2019
f33a34b
removed unused repolink var
bhalbright Aug 17, 2019
6647aa8
fixed query that was breaking ci tests; fixed check in issue dependen…
bhalbright Aug 18, 2019
f1bae29
reverted removal of string in local files becasue these are done via …
bhalbright Aug 19, 2019
935eae4
removed 'Select("issue.*")' from getBlockedByDependencies and getBloc…
bhalbright Aug 20, 2019
0473eca
changed getBlockedByDependencies and getBlockingDependencies to use a…
bhalbright Aug 21, 2019
52f0e19
simplified the getBlockingDependencies and getBlockedByDependencies m…
bhalbright Aug 24, 2019
0b9e67b
made some changes to the issue view in the dependencies (issue name o…
bhalbright Aug 25, 2019
b426606
replace call to FindUserAccessibleRepoIDs with SearchRepositoryByName…
bhalbright Sep 2, 2019
2f257d0
some more tweaks to the layout of the issues when showing dependencie…
bhalbright Sep 3, 2019
3c78743
Merge branch 'master' of https://github.com/go-gitea/gitea
bhalbright Sep 3, 2019
662cf44
added Name to the RepositoryMeta struct
bhalbright Sep 8, 2019
4e90f86
Merge branch 'master' of https://github.com/go-gitea/gitea
bhalbright Sep 8, 2019
6d741b7
updated swagger doc
bhalbright Sep 8, 2019
cf015b6
fixed total count for link header on SearchIssues
bhalbright Sep 8, 2019
e630aa7
fixed indentation
bhalbright Sep 8, 2019
2af7328
Merge branch 'master' of https://github.com/go-gitea/gitea
bhalbright Sep 11, 2019
1b08937
Merge branch 'master' of https://github.com/go-gitea/gitea
bhalbright Sep 12, 2019
3a5f40b
Merge branch 'master' of https://github.com/go-gitea/gitea
bhalbright Sep 25, 2019
deb2ff3
fixed aligment of remove icon on dependencies in issue sidebar
bhalbright Sep 26, 2019
b7cd0f5
removed unnecessary nil check (unnecessary because issue.loadRepo is …
bhalbright Sep 28, 2019
a707ea7
reverting .css change, somehow missed or forgot that less is used
bhalbright Sep 28, 2019
25c40a4
updated less file and generated css; updated sidebar template with st…
bhalbright Sep 28, 2019
3a9028f
added ordering to the blocked by/depends on queries
bhalbright Sep 28, 2019
9dd2fc0
fixed sorting in issue dependency search and the depends on/blocks vi…
bhalbright Oct 6, 2019
e629f7e
Merge branch 'master' of https://github.com/go-gitea/gitea
bhalbright Oct 6, 2019
0327365
re-applied my swagger changes after merge
bhalbright Oct 6, 2019
17b7888
fixed split string condition in issue search
bhalbright Oct 6, 2019
2ab1058
Merge branch 'master' of https://github.com/go-gitea/gitea
bhalbright Oct 8, 2019
690442c
changed ALLOW_CROSS_REPOSITORY_DEPENDENCIES description to sound more…
bhalbright Oct 8, 2019
cf36916
Merge branch 'master' of https://github.com/go-gitea/gitea
bhalbright Oct 8, 2019
ea04606
Merge branch 'master' of https://github.com/go-gitea/gitea
bhalbright Oct 12, 2019
49b7d2a
Merge branch 'master' of https://github.com/go-gitea/gitea
bhalbright Oct 19, 2019
2ec084d
Merge branch 'master' of https://github.com/go-gitea/gitea
bhalbright Oct 26, 2019
c77a950
when adding a dependency to an issue, added a check to make sure the …
bhalbright Oct 26, 2019
c19468c
Merge branch 'master' of https://github.com/go-gitea/gitea
bhalbright Oct 29, 2019
171465c
Merge branch 'master' into master
lunny Oct 30, 2019
50680e4
Merge branch 'master' into master
techknowlogick Oct 30, 2019
074edcf
Merge branch 'master' into master
zeripath Oct 30, 2019
ddd3488
Merge branch 'master' of https://github.com/go-gitea/gitea
bhalbright Oct 31, 2019
1d8bd09
updated sortIssuesSession call in PullRequests, another commit moved …
bhalbright Oct 31, 2019
22c5dd2
fixed incorrect setting of user id parameter in search repos call
bhalbright Oct 31, 2019
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
38 changes: 26 additions & 12 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,12 @@ func (issue *Issue) apiFormat(e Engine) *api.Issue {
Updated: issue.UpdatedUnix.AsTime(),
}

apiIssue.Repo = &api.RepositoryMeta{
ID: issue.Repo.ID,
Name: issue.Repo.Name,
FullName: issue.Repo.FullName(),
}

if issue.ClosedUnix != 0 {
apiIssue.Closed = issue.ClosedUnix.AsTimePtr()
}
Expand Down Expand Up @@ -1790,8 +1796,8 @@ func GetRepoIssueStats(repoID, uid int64, filterMode int, isPull bool) (numOpen
}

// SearchIssueIDsByKeyword search issues on database
func SearchIssueIDsByKeyword(kw string, repoID int64, limit, start int) (int64, []int64, error) {
var repoCond = builder.Eq{"repo_id": repoID}
func SearchIssueIDsByKeyword(kw string, repoIDs []int64, limit, start int) (int64, []int64, error) {
var repoCond = builder.In("repo_id", repoIDs)
var subQuery = builder.Select("id").From("issue").Where(repoCond)
bhalbright marked this conversation as resolved.
Show resolved Hide resolved
var cond = builder.And(
repoCond,
Expand Down Expand Up @@ -1880,33 +1886,41 @@ func UpdateIssueDeadline(issue *Issue, deadlineUnix timeutil.TimeStamp, doer *Us
return sess.Commit()
}

// DependencyInfo represents high level information about an issue which is a dependency of another issue.
type DependencyInfo struct {
Issue `xorm:"extends"`
Repository `xorm:"extends"`
bhalbright marked this conversation as resolved.
Show resolved Hide resolved
}

// Get Blocked By Dependencies, aka all issues this issue is blocked by.
func (issue *Issue) getBlockedByDependencies(e Engine) (issueDeps []*Issue, err error) {
func (issue *Issue) getBlockedByDependencies(e Engine) (issueDeps []*DependencyInfo, err error) {
return issueDeps, e.
Table("issue_dependency").
Select("issue.*").
Join("INNER", "issue", "issue.id = issue_dependency.dependency_id").
Table("issue").
Join("INNER", "repository", "repository.id = issue.repo_id").
Join("INNER", "issue_dependency", "issue_dependency.dependency_id = issue.id").
Where("issue_id = ?", issue.ID).
OrderBy("repository.id, issue.id ASC").
Copy link
Member

Choose a reason for hiding this comment

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

Mmm.... If you sort by issue.id DESC you get the newer issues first, which would make sense. Order by repository ID makes no sense. It's an internal number.
Perhaps:

OrderBy("CASE WHEN repository.id = ? THEN 1 ELSE 2 END, issue.id DESC", current-repo-id)

would cause the issues from the current repo to show first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gotcha, these are for the list of blocked by or blocking dependencies displayed on the issue page (i.e. not the drop-down list to add a new one). I was thinking it would make sense for the issues to be in order of their id grouped by repo, so you'd see all the "repo1" issues and then "repo2" issues, not mixed together. But I agree with your suggestion, thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see! To group by repo and still have the current repo top of the list:

OrderBy("CASE WHEN repository.id = ? THEN 0 ELSE repository.id END, issue.id DESC", current-repo-id)

But I've got confused. I thought this was for the search function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the suggestion, I used the order by statement you suggested

Find(&issueDeps)
bhalbright marked this conversation as resolved.
Show resolved Hide resolved
}

// Get Blocking Dependencies, aka all issues this issue blocks.
func (issue *Issue) getBlockingDependencies(e Engine) (issueDeps []*Issue, err error) {
func (issue *Issue) getBlockingDependencies(e Engine) (issueDeps []*DependencyInfo, err error) {
return issueDeps, e.
Table("issue_dependency").
Select("issue.*").
Join("INNER", "issue", "issue.id = issue_dependency.issue_id").
Table("issue").
Join("INNER", "repository", "repository.id = issue.repo_id").
Join("INNER", "issue_dependency", "issue_dependency.issue_id = issue.id").
Where("dependency_id = ?", issue.ID).
OrderBy("repository.id, issue.id ASC").
Find(&issueDeps)
bhalbright marked this conversation as resolved.
Show resolved Hide resolved
}

// BlockedByDependencies finds all Dependencies an issue is blocked by
func (issue *Issue) BlockedByDependencies() ([]*Issue, error) {
func (issue *Issue) BlockedByDependencies() ([]*DependencyInfo, error) {
return issue.getBlockedByDependencies(x)
}

// BlockingDependencies returns all blocking dependencies, aka all other issues a given issue blocks
func (issue *Issue) BlockingDependencies() ([]*Issue, error) {
func (issue *Issue) BlockingDependencies() ([]*DependencyInfo, error) {
return issue.getBlockingDependencies(x)
}

Expand Down
13 changes: 13 additions & 0 deletions models/issue_label.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,19 @@ func GetLabelIDsInRepoByNames(repoID int64, labelNames []string) ([]int64, error
Find(&labelIDs)
}

// GetLabelIDsInReposByNames returns a list of labelIDs by names in one of the given
// repositories.
// it silently ignores label names that do not belong to the repository.
func GetLabelIDsInReposByNames(repoIDs []int64, labelNames []string) ([]int64, error) {
labelIDs := make([]int64, 0, len(labelNames))
return labelIDs, x.Table("label").
In("repo_id", repoIDs).
In("name", labelNames).
Asc("name").
Cols("id").
Find(&labelIDs)
}

// GetLabelInRepoByID returns a label by ID in given repository.
func GetLabelInRepoByID(repoID, labelID int64) (*Label, error) {
return getLabelInRepoByID(x, repoID, labelID)
Expand Down
9 changes: 4 additions & 5 deletions models/issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,24 +298,23 @@ func TestIssue_loadTotalTimes(t *testing.T) {

func TestIssue_SearchIssueIDsByKeyword(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())

total, ids, err := SearchIssueIDsByKeyword("issue2", 1, 10, 0)
total, ids, err := SearchIssueIDsByKeyword("issue2", []int64{1}, 10, 0)
assert.NoError(t, err)
assert.EqualValues(t, 1, total)
assert.EqualValues(t, []int64{2}, ids)

total, ids, err = SearchIssueIDsByKeyword("first", 1, 10, 0)
total, ids, err = SearchIssueIDsByKeyword("first", []int64{1}, 10, 0)
assert.NoError(t, err)
assert.EqualValues(t, 1, total)
assert.EqualValues(t, []int64{1}, ids)

total, ids, err = SearchIssueIDsByKeyword("for", 1, 10, 0)
total, ids, err = SearchIssueIDsByKeyword("for", []int64{1}, 10, 0)
assert.NoError(t, err)
assert.EqualValues(t, 4, total)
assert.EqualValues(t, []int64{1, 2, 3, 5}, ids)

// issue1's comment id 2
total, ids, err = SearchIssueIDsByKeyword("good", 1, 10, 0)
total, ids, err = SearchIssueIDsByKeyword("good", []int64{1}, 10, 0)
assert.NoError(t, err)
assert.EqualValues(t, 1, total)
assert.EqualValues(t, []int64{1}, ids)
Expand Down
16 changes: 12 additions & 4 deletions modules/indexer/issues/bleve.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,18 @@ func (b *BleveIndexer) Delete(ids ...int64) error {

// Search searches for issues by given conditions.
// Returns the matching issue IDs
func (b *BleveIndexer) Search(keyword string, repoID int64, limit, start int) (*SearchResult, error) {
func (b *BleveIndexer) Search(keyword string, repoIDs []int64, limit, start int) (*SearchResult, error) {
var repoQueriesP []*query.NumericRangeQuery
for _, repoID := range repoIDs {
repoQueriesP = append(repoQueriesP, numericEqualityQuery(repoID, "RepoID"))
}
repoQueries := make([]query.Query, len(repoQueriesP))
for i, v := range repoQueriesP {
repoQueries[i] = query.Query(v)
}

indexerQuery := bleve.NewConjunctionQuery(
numericEqualityQuery(repoID, "RepoID"),
bleve.NewDisjunctionQuery(repoQueries...),
bleve.NewDisjunctionQuery(
newMatchPhraseQuery(keyword, "Title", issueIndexerAnalyzer),
newMatchPhraseQuery(keyword, "Content", issueIndexerAnalyzer),
Expand All @@ -242,8 +251,7 @@ func (b *BleveIndexer) Search(keyword string, repoID int64, limit, start int) (*
return nil, err
}
ret.Hits = append(ret.Hits, Match{
ID: id,
RepoID: repoID,
ID: id,
})
}
return &ret, nil
Expand Down
2 changes: 1 addition & 1 deletion modules/indexer/issues/bleve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func TestBleveIndexAndSearch(t *testing.T) {
)

for _, kw := range keywords {
res, err := indexer.Search(kw.Keyword, 2, 10, 0)
res, err := indexer.Search(kw.Keyword, []int64{2}, 10, 0)
assert.NoError(t, err)

var ids = make([]int64, 0, len(res.Hits))
Expand Down
7 changes: 3 additions & 4 deletions modules/indexer/issues/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ func (db *DBIndexer) Delete(ids ...int64) error {
}

// Search dummy function
func (db *DBIndexer) Search(kw string, repoID int64, limit, start int) (*SearchResult, error) {
total, ids, err := models.SearchIssueIDsByKeyword(kw, repoID, limit, start)
func (db *DBIndexer) Search(kw string, repoIDs []int64, limit, start int) (*SearchResult, error) {
total, ids, err := models.SearchIssueIDsByKeyword(kw, repoIDs, limit, start)
if err != nil {
return nil, err
}
Expand All @@ -37,8 +37,7 @@ func (db *DBIndexer) Search(kw string, repoID int64, limit, start int) (*SearchR
}
for _, id := range ids {
result.Hits = append(result.Hits, Match{
ID: id,
RepoID: repoID,
ID: id,
})
}
return &result, nil
Expand Down
11 changes: 5 additions & 6 deletions modules/indexer/issues/indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,8 @@ type IndexerData struct {

// Match represents on search result
type Match struct {
ID int64 `json:"id"`
RepoID int64 `json:"repo_id"`
Score float64 `json:"score"`
ID int64 `json:"id"`
Score float64 `json:"score"`
}

// SearchResult represents search results
Expand All @@ -42,7 +41,7 @@ type Indexer interface {
Init() (bool, error)
Index(issue []*IndexerData) error
Delete(ids ...int64) error
Search(kw string, repoID int64, limit, start int) (*SearchResult, error)
Search(kw string, repoIDs []int64, limit, start int) (*SearchResult, error)
}

var (
Expand Down Expand Up @@ -195,9 +194,9 @@ func DeleteRepoIssueIndexer(repo *models.Repository) {
}

// SearchIssuesByKeyword search issue ids by keywords and repo id
func SearchIssuesByKeyword(repoID int64, keyword string) ([]int64, error) {
func SearchIssuesByKeyword(repoIDs []int64, keyword string) ([]int64, error) {
var issueIDs []int64
res, err := issueIndexer.Search(keyword, repoID, 1000, 0)
res, err := issueIndexer.Search(keyword, repoIDs, 1000, 0)
if err != nil {
return nil, err
}
Expand Down
16 changes: 8 additions & 8 deletions modules/indexer/issues/indexer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,19 @@ func TestBleveSearchIssues(t *testing.T) {

time.Sleep(5 * time.Second)

ids, err := SearchIssuesByKeyword(1, "issue2")
ids, err := SearchIssuesByKeyword([]int64{1}, "issue2")
assert.NoError(t, err)
assert.EqualValues(t, []int64{2}, ids)

ids, err = SearchIssuesByKeyword(1, "first")
ids, err = SearchIssuesByKeyword([]int64{1}, "first")
assert.NoError(t, err)
assert.EqualValues(t, []int64{1}, ids)

ids, err = SearchIssuesByKeyword(1, "for")
ids, err = SearchIssuesByKeyword([]int64{1}, "for")
assert.NoError(t, err)
assert.EqualValues(t, []int64{1, 2, 3, 5}, ids)

ids, err = SearchIssuesByKeyword(1, "good")
ids, err = SearchIssuesByKeyword([]int64{1}, "good")
assert.NoError(t, err)
assert.EqualValues(t, []int64{1}, ids)
}
Expand All @@ -63,19 +63,19 @@ func TestDBSearchIssues(t *testing.T) {
fatalTestError("Error InitIssueIndexer: %v\n", err)
}

ids, err := SearchIssuesByKeyword(1, "issue2")
ids, err := SearchIssuesByKeyword([]int64{1}, "issue2")
assert.NoError(t, err)
assert.EqualValues(t, []int64{2}, ids)

ids, err = SearchIssuesByKeyword(1, "first")
ids, err = SearchIssuesByKeyword([]int64{1}, "first")
assert.NoError(t, err)
assert.EqualValues(t, []int64{1}, ids)

ids, err = SearchIssuesByKeyword(1, "for")
ids, err = SearchIssuesByKeyword([]int64{1}, "for")
assert.NoError(t, err)
assert.EqualValues(t, []int64{1, 2, 3, 5}, ids)

ids, err = SearchIssuesByKeyword(1, "good")
ids, err = SearchIssuesByKeyword([]int64{1}, "good")
assert.NoError(t, err)
assert.EqualValues(t, []int64{1}, ids)
}
8 changes: 8 additions & 0 deletions modules/structs/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ type PullRequestMeta struct {
Merged *time.Time `json:"merged_at"`
}

// RepositoryMeta basic repository information
type RepositoryMeta struct {
ID int64 `json:"id"`
Name string `json:"name"`
FullName string `json:"full_name"`
}

// Issue represents an issue in a repository
// swagger:model
type Issue struct {
Expand Down Expand Up @@ -57,6 +64,7 @@ type Issue struct {
Deadline *time.Time `json:"due_date"`

PullRequest *PullRequestMeta `json:"pull_request"`
Repo *RepositoryMeta `json:"repository"`
}

// ListIssueOption list issue options
Expand Down
3 changes: 3 additions & 0 deletions public/css/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ a{cursor:pointer}
.ui.form .ui.button{font-weight:400}
.ui.floating.label{z-index:10}
.ui.transparent.label{background-color:transparent}
.ui.nopadding{padding:0}
.ui.menu,.ui.segment,.ui.vertical.menu{box-shadow:none}
.ui .menu:not(.vertical) .item>.button.compact{padding:.58928571em 1.125em}
.ui .menu:not(.vertical) .item>.button.small{font-size:.92857143rem}
Expand Down Expand Up @@ -109,6 +110,8 @@ a{cursor:pointer}
.ui .text.truncate{overflow:hidden;text-overflow:ellipsis;white-space:nowrap;display:inline-block}
.ui .text.thin{font-weight:400}
.ui .text.middle{vertical-align:middle}
.ui .text.nopadding{padding:0}
.ui .text.nomargin{margin:0}
.ui .message{text-align:center}
.ui.bottom.attached.message{font-weight:700;text-align:left;color:#000}
.ui.bottom.attached.message .pull-right{color:#000}
Expand Down
6 changes: 3 additions & 3 deletions public/js/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3096,11 +3096,10 @@ function deleteDependencyModal(id, type) {
}

function initIssueList() {
const repolink = $('#repolink').val();
$('#new-dependency-drop-list')
.dropdown({
apiSettings: {
url: suburl + '/api/v1/repos/' + repolink + '/issues?q={query}',
url: suburl + '/api/v1/repos/issues/search?q={query}',
Copy link
Member

Choose a reason for hiding this comment

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

I think you should pass the repo id or name, and prioritize issues from that repository. There can be thousands of issues in a Gitea installation (Gitea alone has passed 8200).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, this is done now

onResponse: function(response) {
const filteredResponse = {'success': true, 'results': []};
const currIssueId = $('#new-dependency-drop-list').data('issue-id');
Expand All @@ -3111,7 +3110,8 @@ function initIssueList() {
return;
}
filteredResponse.results.push({
'name' : '#' + issue.number + ' ' + htmlEncode(issue.title),
'name' : '#' + issue.number + ' ' + htmlEncode(issue.title) +
'<div class="text small dont-break-out">' + htmlEncode(issue.repository.full_name) + '</div>',
'value' : issue.id
});
});
Expand Down
12 changes: 12 additions & 0 deletions public/less/_base.less
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,10 @@ code,
background-color: transparent;
}

&.nopadding {
padding: 0;
}

&.menu,
&.vertical.menu,
&.segment {
Expand Down Expand Up @@ -453,6 +457,14 @@ code,
&.middle {
vertical-align: middle;
}

&.nopadding {
padding: 0;
}

&.nomargin {
margin: 0;
}
}

.message {
Expand Down
2 changes: 2 additions & 0 deletions routers/api/v1/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,8 @@ func RegisterRoutes(m *macaron.Macaron) {
m.Get("/search", repo.Search)
})

m.Get("/repos/issues/search", repo.SearchIssues)

m.Combo("/repositories/:id", reqToken()).Get(repo.GetByID)

m.Group("/repos", func() {
Expand Down
Loading