git724/forgejo
1
0
Fork 0
Code Issues Pull requests 3 Projects 1 Releases Packages Wiki Activity

Fix issue dependencies (#27736)

Browse source 
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)
This commit is contained in:
Nanguan Lin 2024-01-13 00:49:02 +08:00 • committed by GitHub
commit 9b59af37e7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 55 additions and 56 deletions
Download patch file Download diff file Expand all files Collapse all files

61
routers/api/v1/repo/issue_dependency.go
View file

@ -102,23 +102,24 @@ func GetIssueDependencies(ctx *context.APIContext) {
return return
} }
var lastRepoID int64 repoPerms := make(map[int64]access_model.Permission)
var lastPerm access_model.Permission repoPerms[ctx.Repo.Repository.ID] = ctx.Repo.Permission
for _, blocker := range blockersInfo { for _, blocker := range blockersInfo {
// Get the permissions for this repository // Get the permissions for this repository
perm := lastPerm // If the repo ID exists in the map, return the exist permissions
if lastRepoID != blocker.Repository.ID { // else get the permission and add it to the map
if blocker.Repository.ID == ctx.Repo.Repository.ID { var perm access_model.Permission
perm = ctx.Repo.Permission existPerm, ok := repoPerms[blocker.RepoID]
} else { if ok {
var err error perm = existPerm
perm, err = access_model.GetUserRepoPermission(ctx, &blocker.Repository, ctx.Doer) } else {
if err != nil { var err error
ctx.ServerError("GetUserRepoPermission", err) perm, err = access_model.GetUserRepoPermission(ctx, &blocker.Repository, ctx.Doer)
return if err != nil {
} ctx.ServerError("GetUserRepoPermission", err)
return
} }
lastRepoID = blocker.Repository.ID repoPerms[blocker.RepoID] = perm
} }
// check permission // check permission
@ -345,29 +346,31 @@ func GetIssueBlocks(ctx *context.APIContext) {
return return
} }
var lastRepoID int64
var lastPerm access_model.Permission
var issues []*issues_model.Issue 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 { for i, depMeta := range deps {
if i < skip || i >= max { if i < skip || i >= max {
continue continue
} }
// Get the permissions for this repository // Get the permissions for this repository
perm := lastPerm // If the repo ID exists in the map, return the exist permissions
if lastRepoID != depMeta.Repository.ID { // else get the permission and add it to the map
if depMeta.Repository.ID == ctx.Repo.Repository.ID { var perm access_model.Permission
perm = ctx.Repo.Permission existPerm, ok := repoPerms[depMeta.RepoID]
} else { if ok {
var err error perm = existPerm
perm, err = access_model.GetUserRepoPermission(ctx, &depMeta.Repository, ctx.Doer) } else {
if err != nil { var err error
ctx.ServerError("GetUserRepoPermission", err) perm, err = access_model.GetUserRepoPermission(ctx, &depMeta.Repository, ctx.Doer)
return if err != nil {
} ctx.ServerError("GetUserRepoPermission", err)
return
} }
lastRepoID = depMeta.Repository.ID repoPerms[depMeta.RepoID] = perm
} }
if !perm.CanReadIssuesOrPulls(depMeta.Issue.IsPull) { if !perm.CanReadIssuesOrPulls(depMeta.Issue.IsPull) {

50
routers/web/repo/issue.go
View file

@ -1962,7 +1962,7 @@ func ViewIssue(ctx *context.Context) {
return return
} }
ctx.Data["BlockingDependencies"], ctx.Data["BlockingByDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blocking) ctx.Data["BlockingDependencies"], ctx.Data["BlockingDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blocking)
if ctx.Written() { if ctx.Written() {
return return
} }
@ -2023,38 +2023,34 @@ func ViewIssue(ctx *context.Context) {
// checkBlockedByIssues return canRead and notPermitted // checkBlockedByIssues return canRead and notPermitted
func checkBlockedByIssues(ctx *context.Context, blockers []*issues_model.DependencyInfo) (canRead, notPermitted []*issues_model.DependencyInfo) { func checkBlockedByIssues(ctx *context.Context, blockers []*issues_model.DependencyInfo) (canRead, notPermitted []*issues_model.DependencyInfo) {
var ( repoPerms := make(map[int64]access_model.Permission)
lastRepoID int64 repoPerms[ctx.Repo.Repository.ID] = ctx.Repo.Permission
lastPerm access_model.Permission for _, blocker := range blockers {
)
for i, blocker := range blockers {
// Get the permissions for this repository // Get the permissions for this repository
perm := lastPerm // If the repo ID exists in the map, return the exist permissions
if lastRepoID != blocker.Repository.ID { // else get the permission and add it to the map
if blocker.Repository.ID == ctx.Repo.Repository.ID { var perm access_model.Permission
perm = ctx.Repo.Permission existPerm, ok := repoPerms[blocker.RepoID]
} else { if ok {
var err error perm = existPerm
perm, err = access_model.GetUserRepoPermission(ctx, &blocker.Repository, ctx.Doer) } else {
if err != nil { var err error
ctx.ServerError("GetUserRepoPermission", err) perm, err = access_model.GetUserRepoPermission(ctx, &blocker.Repository, ctx.Doer)
return nil, nil if err != nil {
} ctx.ServerError("GetUserRepoPermission", err)
return nil, nil
} }
lastRepoID = blocker.Repository.ID repoPerms[blocker.RepoID] = perm
} }
if perm.CanReadIssuesOrPulls(blocker.Issue.IsPull) {
// check permission canRead = append(canRead, blocker)
if !perm.CanReadIssuesOrPulls(blocker.Issue.IsPull) { } else {
blockers[len(notPermitted)], blockers[i] = blocker, blockers[len(notPermitted)] notPermitted = append(notPermitted, blocker)
notPermitted = blockers[:len(notPermitted)+1]
} }
} }
blockers = blockers[len(notPermitted):] sortDependencyInfo(canRead)
sortDependencyInfo(blockers)
sortDependencyInfo(notPermitted) sortDependencyInfo(notPermitted)
return canRead, notPermitted
return blockers, notPermitted
} }
func sortDependencyInfo(blockers []*issues_model.DependencyInfo) { func sortDependencyInfo(blockers []*issues_model.DependencyInfo) {