diff options
author | Nanguan Lin <70063547+lng2020@users.noreply.github.com> | 2024-01-13 00:49:02 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-01-12 16:49:02 +0000 |
commit | 9b59af37e72656c71316ec79b5c377a2eb84c671 (patch) | |
tree | a6d7ca55c2e4f91ae6bb2cfba3380031863a82fb | |
parent | 4f8f5f6e251615265392ec4212ed87a58fb1f75e (diff) | |
download | gitea-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)
-rw-r--r-- | routers/api/v1/repo/issue_dependency.go | 61 | ||||
-rw-r--r-- | routers/web/repo/issue.go | 50 |
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) { |