Skip to content

Commit 9b59af3

Browse files
authored
Fix issue dependencies (#27736)
Fix #27722 Fix #27357 Fix #25837 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)
1 parent 4f8f5f6 commit 9b59af3

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
@@ -345,29 +346,31 @@ func GetIssueBlocks(ctx *context.APIContext) {
345346
return
346347
}
347348

348-
var lastRepoID int64
349-
var lastPerm access_model.Permission
350-
351349
var issues []*issues_model.Issue
350+
351+
repoPerms := make(map[int64]access_model.Permission)
352+
repoPerms[ctx.Repo.Repository.ID] = ctx.Repo.Permission
353+
352354
for i, depMeta := range deps {
353355
if i < skip || i >= max {
354356
continue
355357
}
356358

357359
// Get the permissions for this repository
358-
perm := lastPerm
359-
if lastRepoID != depMeta.Repository.ID {
360-
if depMeta.Repository.ID == ctx.Repo.Repository.ID {
361-
perm = ctx.Repo.Permission
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
368-
}
360+
// If the repo ID exists in the map, return the exist permissions
361+
// else get the permission and add it to the map
362+
var perm access_model.Permission
363+
existPerm, ok := repoPerms[depMeta.RepoID]
364+
if ok {
365+
perm = existPerm
366+
} else {
367+
var err error
368+
perm, err = access_model.GetUserRepoPermission(ctx, &depMeta.Repository, ctx.Doer)
369+
if err != nil {
370+
ctx.ServerError("GetUserRepoPermission", err)
371+
return
369372
}
370-
lastRepoID = depMeta.Repository.ID
373+
repoPerms[depMeta.RepoID] = perm
371374
}
372375

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

routers/web/repo/issue.go

+23-27
Original file line numberDiff line numberDiff line change
@@ -1962,7 +1962,7 @@ func ViewIssue(ctx *context.Context) {
19621962
return
19631963
}
19641964

1965-
ctx.Data["BlockingDependencies"], ctx.Data["BlockingByDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blocking)
1965+
ctx.Data["BlockingDependencies"], ctx.Data["BlockingDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blocking)
19661966
if ctx.Written() {
19671967
return
19681968
}
@@ -2023,38 +2023,34 @@ func ViewIssue(ctx *context.Context) {
20232023

20242024
// checkBlockedByIssues return canRead and notPermitted
20252025
func checkBlockedByIssues(ctx *context.Context, blockers []*issues_model.DependencyInfo) (canRead, notPermitted []*issues_model.DependencyInfo) {
2026-
var (
2027-
lastRepoID int64
2028-
lastPerm access_model.Permission
2029-
)
2030-
for i, blocker := range blockers {
2026+
repoPerms := make(map[int64]access_model.Permission)
2027+
repoPerms[ctx.Repo.Repository.ID] = ctx.Repo.Permission
2028+
for _, blocker := range blockers {
20312029
// Get the permissions for this repository
2032-
perm := lastPerm
2033-
if lastRepoID != blocker.Repository.ID {
2034-
if blocker.Repository.ID == ctx.Repo.Repository.ID {
2035-
perm = ctx.Repo.Permission
2036-
} else {
2037-
var err error
2038-
perm, err = access_model.GetUserRepoPermission(ctx, &blocker.Repository, ctx.Doer)
2039-
if err != nil {
2040-
ctx.ServerError("GetUserRepoPermission", err)
2041-
return nil, nil
2042-
}
2030+
// If the repo ID exists in the map, return the exist permissions
2031+
// else get the permission and add it to the map
2032+
var perm access_model.Permission
2033+
existPerm, ok := repoPerms[blocker.RepoID]
2034+
if ok {
2035+
perm = existPerm
2036+
} else {
2037+
var err error
2038+
perm, err = access_model.GetUserRepoPermission(ctx, &blocker.Repository, ctx.Doer)
2039+
if err != nil {
2040+
ctx.ServerError("GetUserRepoPermission", err)
2041+
return nil, nil
20432042
}
2044-
lastRepoID = blocker.Repository.ID
2043+
repoPerms[blocker.RepoID] = perm
20452044
}
2046-
2047-
// check permission
2048-
if !perm.CanReadIssuesOrPulls(blocker.Issue.IsPull) {
2049-
blockers[len(notPermitted)], blockers[i] = blocker, blockers[len(notPermitted)]
2050-
notPermitted = blockers[:len(notPermitted)+1]
2045+
if perm.CanReadIssuesOrPulls(blocker.Issue.IsPull) {
2046+
canRead = append(canRead, blocker)
2047+
} else {
2048+
notPermitted = append(notPermitted, blocker)
20512049
}
20522050
}
2053-
blockers = blockers[len(notPermitted):]
2054-
sortDependencyInfo(blockers)
2051+
sortDependencyInfo(canRead)
20552052
sortDependencyInfo(notPermitted)
2056-
2057-
return blockers, notPermitted
2053+
return canRead, notPermitted
20582054
}
20592055

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

0 commit comments

Comments
 (0)