aboutsummaryrefslogtreecommitdiffstats
path: root/routers
diff options
context:
space:
mode:
authorNanguan Lin <70063547+lng2020@users.noreply.github.com>2024-01-13 00:49:02 +0800
committerGitHub <noreply@github.com>2024-01-12 16:49:02 +0000
commit9b59af37e72656c71316ec79b5c377a2eb84c671 (patch)
treea6d7ca55c2e4f91ae6bb2cfba3380031863a82fb /routers
parent4f8f5f6e251615265392ec4212ed87a58fb1f75e (diff)
downloadgitea-9b59af37e72656c71316ec79b5c377a2eb84c671.tar.gz
gitea-9b59af37e72656c71316ec79b5c377a2eb84c671.zip
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)
Diffstat (limited to 'routers')
-rw-r--r--routers/api/v1/repo/issue_dependency.go61
-rw-r--r--routers/web/repo/issue.go50
2 files changed, 55 insertions, 56 deletions
diff --git a/routers/api/v1/repo/issue_dependency.go b/routers/api/v1/repo/issue_dependency.go
index 0e3bcd93ef..62d1057cdf 100644
--- a/routers/api/v1/repo/issue_dependency.go
+++ b/routers/api/v1/repo/issue_dependency.go
@@ -102,23 +102,24 @@ func GetIssueDependencies(ctx *context.APIContext) {
return
}
- var lastRepoID int64
- var lastPerm access_model.Permission
+ repoPerms := make(map[int64]access_model.Permission)
+ repoPerms[ctx.Repo.Repository.ID] = ctx.Repo.Permission
for _, blocker := range blockersInfo {
// Get the permissions for this repository
- perm := lastPerm
- if lastRepoID != blocker.Repository.ID {
- if blocker.Repository.ID == ctx.Repo.Repository.ID {
- perm = ctx.Repo.Permission
- } else {
- var err error
- perm, err = access_model.GetUserRepoPermission(ctx, &blocker.Repository, ctx.Doer)
- if err != nil {
- ctx.ServerError("GetUserRepoPermission", err)
- return
- }
+ // If the repo ID exists in the map, return the exist permissions
+ // else get the permission and add it to the map
+ var perm access_model.Permission
+ existPerm, ok := repoPerms[blocker.RepoID]
+ if ok {
+ perm = existPerm
+ } else {
+ var err error
+ perm, err = access_model.GetUserRepoPermission(ctx, &blocker.Repository, ctx.Doer)
+ if err != nil {
+ ctx.ServerError("GetUserRepoPermission", err)
+ return
}
- lastRepoID = blocker.Repository.ID
+ repoPerms[blocker.RepoID] = perm
}
// check permission
@@ -345,29 +346,31 @@ func GetIssueBlocks(ctx *context.APIContext) {
return
}
- var lastRepoID int64
- var lastPerm access_model.Permission
-
var issues []*issues_model.Issue
+
+ repoPerms := make(map[int64]access_model.Permission)
+ repoPerms[ctx.Repo.Repository.ID] = ctx.Repo.Permission
+
for i, depMeta := range deps {
if i < skip || i >= max {
continue
}
// Get the permissions for this repository
- perm := lastPerm
- if lastRepoID != depMeta.Repository.ID {
- if depMeta.Repository.ID == ctx.Repo.Repository.ID {
- perm = ctx.Repo.Permission
- } else {
- var err error
- perm, err = access_model.GetUserRepoPermission(ctx, &depMeta.Repository, ctx.Doer)
- if err != nil {
- ctx.ServerError("GetUserRepoPermission", err)
- return
- }
+ // If the repo ID exists in the map, return the exist permissions
+ // else get the permission and add it to the map
+ var perm access_model.Permission
+ existPerm, ok := repoPerms[depMeta.RepoID]
+ if ok {
+ perm = existPerm
+ } else {
+ var err error
+ perm, err = access_model.GetUserRepoPermission(ctx, &depMeta.Repository, ctx.Doer)
+ if err != nil {
+ ctx.ServerError("GetUserRepoPermission", err)
+ return
}
- lastRepoID = depMeta.Repository.ID
+ repoPerms[depMeta.RepoID] = perm
}
if !perm.CanReadIssuesOrPulls(depMeta.Issue.IsPull) {
diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go
index 14781be822..2b5ab21172 100644
--- a/routers/web/repo/issue.go
+++ b/routers/web/repo/issue.go
@@ -1962,7 +1962,7 @@ func ViewIssue(ctx *context.Context) {
return
}
- ctx.Data["BlockingDependencies"], ctx.Data["BlockingByDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blocking)
+ ctx.Data["BlockingDependencies"], ctx.Data["BlockingDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blocking)
if ctx.Written() {
return
}
@@ -2023,38 +2023,34 @@ func ViewIssue(ctx *context.Context) {
// checkBlockedByIssues return canRead and notPermitted
func checkBlockedByIssues(ctx *context.Context, blockers []*issues_model.DependencyInfo) (canRead, notPermitted []*issues_model.DependencyInfo) {
- var (
- lastRepoID int64
- lastPerm access_model.Permission
- )
- for i, blocker := range blockers {
+ repoPerms := make(map[int64]access_model.Permission)
+ repoPerms[ctx.Repo.Repository.ID] = ctx.Repo.Permission
+ for _, blocker := range blockers {
// Get the permissions for this repository
- perm := lastPerm
- if lastRepoID != blocker.Repository.ID {
- if blocker.Repository.ID == ctx.Repo.Repository.ID {
- perm = ctx.Repo.Permission
- } else {
- var err error
- perm, err = access_model.GetUserRepoPermission(ctx, &blocker.Repository, ctx.Doer)
- if err != nil {
- ctx.ServerError("GetUserRepoPermission", err)
- return nil, nil
- }
+ // If the repo ID exists in the map, return the exist permissions
+ // else get the permission and add it to the map
+ var perm access_model.Permission
+ existPerm, ok := repoPerms[blocker.RepoID]
+ if ok {
+ perm = existPerm
+ } else {
+ var err error
+ perm, err = access_model.GetUserRepoPermission(ctx, &blocker.Repository, ctx.Doer)
+ if err != nil {
+ ctx.ServerError("GetUserRepoPermission", err)
+ return nil, nil
}
- lastRepoID = blocker.Repository.ID
+ repoPerms[blocker.RepoID] = perm
}
-
- // check permission
- if !perm.CanReadIssuesOrPulls(blocker.Issue.IsPull) {
- blockers[len(notPermitted)], blockers[i] = blocker, blockers[len(notPermitted)]
- notPermitted = blockers[:len(notPermitted)+1]
+ if perm.CanReadIssuesOrPulls(blocker.Issue.IsPull) {
+ canRead = append(canRead, blocker)
+ } else {
+ notPermitted = append(notPermitted, blocker)
}
}
- blockers = blockers[len(notPermitted):]
- sortDependencyInfo(blockers)
+ sortDependencyInfo(canRead)
sortDependencyInfo(notPermitted)
-
- return blockers, notPermitted
+ return canRead, notPermitted
}
func sortDependencyInfo(blockers []*issues_model.DependencyInfo) {