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.

Sign up for GitHub

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

Jump to bottom

Return empty when searching issues with no repos #26545

Merged
merged 4 commits into from
Aug 17, 2023
Merged

Return empty when searching issues with no repos #26545

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ef16356
fix: do not return all issues
wolfogre Aug 16, 2023
f7a6e16
fix: state
wolfogre Aug 16, 2023
7b55982
chore: revert FIXME
wolfogre Aug 17, 2023
d6471a4
Merge branch 'main' into bugfix/team_issue_indexer
GiteaBot Aug 17, 2023
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
4 changes: 4 additions & 0 deletions routers/api/v1/repo/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,10 @@ func SearchIssues(ctx *context.APIContext) {
ctx.Error(http.StatusInternalServerError, "SearchRepositoryIDs", err)
return
}
if len(repoIDs) == 0 {
// no repos found, don't let the indexer return all repos
repoIDs = []int64{0}
}
}

keyword := ctx.FormTrim("q")
Expand Down
4 changes: 4 additions & 0 deletions routers/web/repo/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2503,6 +2503,10 @@ func SearchIssues(ctx *context.Context) {
ctx.Error(http.StatusInternalServerError, "SearchRepositoryIDs", err.Error())
return
}
if len(repoIDs) == 0 {
// no repos found, don't let the indexer return all repos
repoIDs = []int64{0}
}
}

keyword := ctx.FormTrim("q")
Expand Down
20 changes: 15 additions & 5 deletions routers/web/user/home.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,10 +454,21 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
AllPublic: false,
AllLimited: false,
}

if team != nil {
repoOpts.TeamID = team.ID
}
{
ids, _, err := repo_model.SearchRepositoryIDs(repoOpts)
if err != nil {
ctx.ServerError("SearchRepositoryIDs", err)
return
}
opts.RepoIDs = ids
if len(opts.RepoIDs) == 0 {
// no repos found, don't let the indexer return all repos
opts.RepoIDs = []int64{0}
}
}
Copy link
Member

@silverwind silverwind Aug 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Curious, what's this {} syntax? Is this a on-demand closure like in JS?

Copy link
Member

@jolheiser jolheiser Aug 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It's an int64 slice literal with a single value 0 in it.

Copy link
Member

Choose a reason for hiding this comment

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

Not that one, I meant the enclosing {} between lines 460 and 471.

Copy link
Member

@silverwind silverwind Aug 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Found it, it's called Block: https://go.dev/ref/spec#Blocks, same as in JS, https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/block

Copy link
Member

Choose a reason for hiding this comment

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

Ah, my bad! Yep, a block.

Copy link
Member Author

Choose a reason for hiding this comment

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

Never mind, it is just a trick. I usually do this in a relatively separate piece of code, to avoid polluting a broader scope with local variables, especially when the function body is large.

buildIssueOverview is so big, it has almost 400 lines and at least 20 local variables.

Explicit blocks also make it clear how to refactor them to multiple functions, but I'm too lazy to refactor it right now 😁


switch filterMode {
case issues_model.FilterModeAll:
Expand Down Expand Up @@ -541,15 +552,13 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
// Parse ctx.FormString("repos") and remember matched repo IDs for later.
// Gets set when clicking filters on the issues overview page.
repoIDs := getRepoIDs(ctx.FormString("repos"))
if len(repoIDs) == 0 {
repoIDs = accessibleRepos.Values()
} else {
if len(repoIDs) > 0 {
// Remove repo IDs that are not accessible to the user.
repoIDs = util.SliceRemoveAllFunc(repoIDs, func(v int64) bool {
return !accessibleRepos.Contains(v)
})
opts.RepoIDs = repoIDs
}
opts.RepoIDs = repoIDs

// ------------------------------
// Get issues as defined by opts.
Expand Down Expand Up @@ -609,6 +618,7 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
var issueStats *issues_model.IssueStats
{
statsOpts := issues_model.IssuesOptions{
RepoIDs: repoIDs,
User: ctx.Doer,
IsPull: util.OptionalBoolOf(isPullList),
IsClosed: util.OptionalBoolOf(isShowClosed),
Expand Down