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

Fix the duplicated repositories load on dashboard #29878

Closed
wants to merge 3 commits into from
Closed
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
35 changes: 28 additions & 7 deletions routers/web/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -622,26 +622,47 @@ func SearchRepo(ctx *context.Context) {
}
}

var err error
getErrorStr := func(ctx *context.Context, err error, fallbackStr string) string {
if ctx.Doer == nil || !ctx.Doer.IsAdmin {
return fallbackStr
}
return fmt.Sprintf("%s: %v", fallbackStr, err)
}

// To improve performance when only the count is requested
if ctx.FormBool("count_only") {
count, err := repo_model.CountRepository(ctx, opts)
if err != nil {
log.Error("CountRepository: %v", err)
ctx.JSON(http.StatusInternalServerError, api.SearchError{
OK: false,
Error: getErrorStr(ctx, err, "CountRepository internal error"),
})
return
}
ctx.SetTotalCountHeader(count)
return
}

repos, count, err := repo_model.SearchRepository(ctx, opts)
if err != nil {
log.Error("SearchRepository: %v", err)
ctx.JSON(http.StatusInternalServerError, api.SearchError{
OK: false,
Error: err.Error(),
Error: getErrorStr(ctx, err, "SearchRepository internal error"),
Copy link
Contributor

Choose a reason for hiding this comment

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

getErrorStr makes the code more hacky. Would the code be filled with a lot of getErrorStr in every handler?

And I would ask the question a third time:

And, I don't see why it should use api.SearchError here, is the response really handled by frontend JS?

If I didn't misread the code, the frontend code NEVER handles the response error. It doens't need to respond anything here.

Copy link
Contributor

Choose a reason for hiding this comment

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

-> Only do counting when count_only=true for repo dashboard #29884

What do you think about this? It is simple enough and could be backported.

})
return
}

ctx.SetTotalCountHeader(count)

// To improve performance when only the count is requested
if ctx.FormBool("count_only") {
return
}

latestCommitStatuses, err := commitstatus_service.FindReposLastestCommitStatuses(ctx, repos)
if err != nil {
log.Error("FindReposLastestCommitStatuses: %v", err)
ctx.JSON(http.StatusInternalServerError, api.SearchError{
OK: false,
Error: getErrorStr(ctx, err, "FindReposLastestCommitStatuses internal error"),
})
return
}

Expand Down