Skip to content

Commit 571822b

Browse files
GiteaBotlng2020
andauthored
Fix issue dependencies (#27736) (#28776)
Backport #27736 by @lng2020 Fix #27722 Fix #27357 Fix #25837 Fix #28732 1. Fix the typo `BlockingByDependenciesNotPermitted`, which causes the `not permitted message` not to show. The correct one is `Blocking` or `BlockedBy` 2. Rewrite the perm check. The perm check uses a very tricky way to avoid duplicate checks for a slice of issues, which is confusing. In fact, it's also the reason causing the bug. It uses `lastRepoID` and `lastPerm` to avoid duplicate checks, but forgets to assign the `lastPerm` at the end of the code block. So I rewrote this to avoid this trick. ![I U1AT{GNFY3 1HZ`6L{(2L](https://github.com/go-gitea/gitea/assets/70063547/79acd02a-a567-4316-ae0d-11c6461becf1) 3. It also reuses the `blocks` slice, which is even more confusing. So I rewrote this too. ![UARFPXRGGZQFB7J$2`R}5_R](https://github.com/go-gitea/gitea/assets/70063547/f21cff0f-d9ac-4ce4-ae4d-adffc98ecd99) Co-authored-by: Nanguan Lin <70063547+lng2020@users.noreply.github.com>
1 parent 2a0fbe2 commit 571822b

File tree

2 files changed

+55
-56
lines changed

2 files changed

+55
-56
lines changed

routers/api/v1/repo/issue_dependency.go

+32-29
Original file line numberDiff line numberDiff line change
@@ -102,23 +102,24 @@ func GetIssueDependencies(ctx *context.APIContext) {
102102
return
103103
}
104104

105-
var lastRepoID int64
106-
var lastPerm access_model.Permission
105+
repoPerms := make(map[int64]access_model.Permission)
106+
repoPerms[ctx.Repo.Repository.ID] = ctx.Repo.Permission
107107
for _, blocker := range blockersInfo {
108108
// Get the permissions for this repository
109-
perm := lastPerm
110-
if lastRepoID != blocker.Repository.ID {
111-
if blocker.Repository.ID == ctx.Repo.Repository.ID {
112-
perm = ctx.Repo.Permission
113-
} else {
114-
var err error
115-
perm, err = access_model.GetUserRepoPermission(ctx, &blocker.Repository, ctx.Doer)
116-
if err != nil {
117-
ctx.ServerError("GetUserRepoPermission", err)
118-
return
119-
}
109+
// If the repo ID exists in the map, return the exist permissions
110+
// else get the permission and add it to the map
111+
var perm access_model.Permission
112+
existPerm, ok := repoPerms[blocker.RepoID]
113+
if ok {
114+
perm = existPerm
115+
} else {
116+
var err error
117+
perm, err = access_model.GetUserRepoPermission(ctx, &blocker.Repository, ctx.Doer)
118+
if err != nil {
119+
ctx.ServerError("GetUserRepoPermission", err)
120+
return
120121
}
121-
lastRepoID = blocker.Repository.ID
122+
repoPerms[blocker.RepoID] = perm
122123
}
123124

124125
// check permission
@@ -341,29 +342,31 @@ func GetIssueBlocks(ctx *context.APIContext) {
341342
return
342343
}
343344

344-
var lastRepoID int64
345-
var lastPerm access_model.Permission
346-
347345
var issues []*issues_model.Issue
346+
347+
repoPerms := make(map[int64]access_model.Permission)
348+
repoPerms[ctx.Repo.Repository.ID] = ctx.Repo.Permission
349+
348350
for i, depMeta := range deps {
349351
if i < skip || i >= max {
350352
continue
351353
}
352354

353355
// Get the permissions for this repository
354-
perm := lastPerm
355-
if lastRepoID != depMeta.Repository.ID {
356-
if depMeta.Repository.ID == ctx.Repo.Repository.ID {
357-
perm = ctx.Repo.Permission
358-
} else {
359-
var err error
360-
perm, err = access_model.GetUserRepoPermission(ctx, &depMeta.Repository, ctx.Doer)
361-
if err != nil {
362-
ctx.ServerError("GetUserRepoPermission", err)
363-
return
364-
}
356+
// If the repo ID exists in the map, return the exist permissions
357+
// else get the permission and add it to the map
358+
var perm access_model.Permission
359+
existPerm, ok := repoPerms[depMeta.RepoID]
360+
if ok {
361+
perm = existPerm
362+
} else {
363+
var err error
364+
perm, err = access_model.GetUserRepoPermission(ctx, &depMeta.Repository, ctx.Doer)
365+
if err != nil {
366+
ctx.ServerError("GetUserRepoPermission", err)
367+
return
365368
}
366-
lastRepoID = depMeta.Repository.ID
369+
repoPerms[depMeta.RepoID] = perm
367370
}
368371

369372
if !perm.CanReadIssuesOrPulls(depMeta.Issue.IsPull) {

routers/web/repo/issue.go

+23-27
Original file line numberDiff line numberDiff line change
@@ -1948,7 +1948,7 @@ func ViewIssue(ctx *context.Context) {
19481948
return
19491949
}
19501950

1951-
ctx.Data["BlockingDependencies"], ctx.Data["BlockingByDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blocking)
1951+
ctx.Data["BlockingDependencies"], ctx.Data["BlockingDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blocking)
19521952
if ctx.Written() {
19531953
return
19541954
}
@@ -2009,38 +2009,34 @@ func ViewIssue(ctx *context.Context) {
20092009

20102010
// checkBlockedByIssues return canRead and notPermitted
20112011
func checkBlockedByIssues(ctx *context.Context, blockers []*issues_model.DependencyInfo) (canRead, notPermitted []*issues_model.DependencyInfo) {
2012-
var (
2013-
lastRepoID int64
2014-
lastPerm access_model.Permission
2015-
)
2016-
for i, blocker := range blockers {
2012+
repoPerms := make(map[int64]access_model.Permission)
2013+
repoPerms[ctx.Repo.Repository.ID] = ctx.Repo.Permission
2014+
for _, blocker := range blockers {
20172015
// Get the permissions for this repository
2018-
perm := lastPerm
2019-
if lastRepoID != blocker.Repository.ID {
2020-
if blocker.Repository.ID == ctx.Repo.Repository.ID {
2021-
perm = ctx.Repo.Permission
2022-
} else {
2023-
var err error
2024-
perm, err = access_model.GetUserRepoPermission(ctx, &blocker.Repository, ctx.Doer)
2025-
if err != nil {
2026-
ctx.ServerError("GetUserRepoPermission", err)
2027-
return nil, nil
2028-
}
2016+
// If the repo ID exists in the map, return the exist permissions
2017+
// else get the permission and add it to the map
2018+
var perm access_model.Permission
2019+
existPerm, ok := repoPerms[blocker.RepoID]
2020+
if ok {
2021+
perm = existPerm
2022+
} else {
2023+
var err error
2024+
perm, err = access_model.GetUserRepoPermission(ctx, &blocker.Repository, ctx.Doer)
2025+
if err != nil {
2026+
ctx.ServerError("GetUserRepoPermission", err)
2027+
return nil, nil
20292028
}
2030-
lastRepoID = blocker.Repository.ID
2029+
repoPerms[blocker.RepoID] = perm
20312030
}
2032-
2033-
// check permission
2034-
if !perm.CanReadIssuesOrPulls(blocker.Issue.IsPull) {
2035-
blockers[len(notPermitted)], blockers[i] = blocker, blockers[len(notPermitted)]
2036-
notPermitted = blockers[:len(notPermitted)+1]
2031+
if perm.CanReadIssuesOrPulls(blocker.Issue.IsPull) {
2032+
canRead = append(canRead, blocker)
2033+
} else {
2034+
notPermitted = append(notPermitted, blocker)
20372035
}
20382036
}
2039-
blockers = blockers[len(notPermitted):]
2040-
sortDependencyInfo(blockers)
2037+
sortDependencyInfo(canRead)
20412038
sortDependencyInfo(notPermitted)
2042-
2043-
return blockers, notPermitted
2039+
return canRead, notPermitted
20442040
}
20452041

20462042
func sortDependencyInfo(blockers []*issues_model.DependencyInfo) {

0 commit comments

Comments
 (0)