From fadaf36e95381213c17d066dc4836c793a0d8483 Mon Sep 17 00:00:00 2001 From: Shudhanshu Singh Date: Sat, 25 Jul 2026 18:09:36 +0530 Subject: [PATCH] fix(repo): prevent double-write redirect collisions on dependency errors, fix ui (#38627) 1. fix `AddDependency` and ` RemoveDependency` to respond correctly 2. refactor the form to use "form-fetch-action" 3. fix the issue dependency icon layout regression --------- Signed-off-by: Sudhanshu Singh Co-authored-by: wxiaoguang --- routers/web/repo/issue_dependency.go | 51 ++++++++----------- .../issue/sidebar/issue_dependencies.tmpl | 26 +++++----- .../integration/api_issue_dependency_test.go | 18 +++---- web_src/css/repo.css | 7 +++ 4 files changed, 48 insertions(+), 54 deletions(-) diff --git a/routers/web/repo/issue_dependency.go b/routers/web/repo/issue_dependency.go index 7570147fc4..1223f1868c 100644 --- a/routers/web/repo/issue_dependency.go +++ b/routers/web/repo/issue_dependency.go @@ -4,8 +4,6 @@ package repo import ( - "net/http" - issues_model "gitea.dev/models/issues" access_model "gitea.dev/models/perm/access" "gitea.dev/modules/setting" @@ -23,7 +21,7 @@ func AddDependency(ctx *context.Context) { // Check if the Repo is allowed to have dependencies if !ctx.Repo.CanCreateIssueDependencies(ctx, ctx.Doer, issue.IsPull) { - ctx.HTTPError(http.StatusForbidden, "CanCreateIssueDependencies") + ctx.JSONError(ctx.Locale.TrString("error.permission_denied")) return } @@ -34,20 +32,17 @@ func AddDependency(ctx *context.Context) { return } - // Redirect - defer ctx.Redirect(issue.Link()) - // Dependency dep, err := issues_model.GetIssueByID(ctx, depID) if err != nil { - ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_issue_not_exist")) + ctx.JSONError(ctx.Tr("repo.issues.dependency.add_error_dep_issue_not_exist")) return } // Check if both issues are in the same repo if cross repository dependencies is not enabled if issue.RepoID != dep.RepoID { if !setting.Service.AllowCrossRepositoryDependencies { - ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_not_same_repo")) + ctx.JSONError(ctx.Tr("repo.issues.dependency.add_error_dep_not_same_repo")) return } if err := dep.LoadRepo(ctx); err != nil { @@ -61,29 +56,29 @@ func AddDependency(ctx *context.Context) { return } if !depRepoPerm.CanReadIssuesOrPulls(dep.IsPull) { - // you can't see this dependency + ctx.JSONError(ctx.Locale.TrString("error.permission_denied")) return } } // Check if issue and dependency is the same if dep.ID == issue.ID { - ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_same_issue")) + ctx.JSONError(ctx.Tr("repo.issues.dependency.add_error_same_issue")) return } err = issues_model.CreateIssueDependency(ctx, ctx.Doer, issue, dep) - if err != nil { - if issues_model.IsErrDependencyExists(err) { - ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_exists")) - return - } else if issues_model.IsErrCircularDependency(err) { - ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_cannot_create_circular")) - return - } + if issues_model.IsErrDependencyExists(err) { + ctx.JSONError(ctx.Tr("repo.issues.dependency.add_error_dep_exists")) + return + } else if issues_model.IsErrCircularDependency(err) { + ctx.JSONError(ctx.Tr("repo.issues.dependency.add_error_cannot_create_circular")) + return + } else if err != nil { ctx.ServerError("CreateOrUpdateIssueDependency", err) return } + ctx.JSONOK() } // RemoveDependency removes the dependency @@ -97,7 +92,7 @@ func RemoveDependency(ctx *context.Context) { // Check if the Repo is allowed to have dependencies if !ctx.Repo.CanCreateIssueDependencies(ctx, ctx.Doer, issue.IsPull) { - ctx.HTTPError(http.StatusForbidden, "CanCreateIssueDependencies") + ctx.JSONError(ctx.Locale.TrString("error.permission_denied")) return } @@ -119,7 +114,7 @@ func RemoveDependency(ctx *context.Context) { case "blocking": depType = issues_model.DependencyTypeBlocking default: - ctx.HTTPError(http.StatusBadRequest, "GetDependencyType") + ctx.JSONError("invalid dependency type") return } @@ -144,20 +139,18 @@ func RemoveDependency(ctx *context.Context) { return } if !depRepoPerm.CanReadIssuesOrPulls(dep.IsPull) { - ctx.Redirect(issue.Link()) + ctx.JSONError(ctx.Locale.TrString("error.permission_denied")) return } } - if err = issues_model.RemoveIssueDependency(ctx, ctx.Doer, issue, dep, depType); err != nil { - if issues_model.IsErrDependencyNotExists(err) { - ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_not_exist")) - return - } + err = issues_model.RemoveIssueDependency(ctx, ctx.Doer, issue, dep, depType) + if issues_model.IsErrDependencyNotExists(err) { + ctx.JSONError(ctx.Tr("repo.issues.dependency.add_error_dep_not_exist")) + return + } else if err != nil { ctx.ServerError("RemoveIssueDependency", err) return } - - // Redirect - ctx.Redirect(issue.Link()) + ctx.JSONOK() } diff --git a/templates/repo/issue/sidebar/issue_dependencies.tmpl b/templates/repo/issue/sidebar/issue_dependencies.tmpl index f1555dfa39..58dd61fb69 100644 --- a/templates/repo/issue/sidebar/issue_dependencies.tmpl +++ b/templates/repo/issue/sidebar/issue_dependencies.tmpl @@ -18,10 +18,10 @@ {{ctx.Locale.Tr "repo.issues.dependency.blocks_short"}} -
+
{{range .BlockingDependencies}} -
-
+
+
#{{.Issue.Index}} {{.Issue.Title | ctx.RenderUtils.RenderEmoji}} @@ -29,7 +29,7 @@ {{.Repository.OwnerName}}/{{.Repository.Name}}
-
+
{{if and $.CanCreateIssueDependencies (not $.Repository.IsArchived)}} {{ctx.Locale.Tr "repo.issues.dependency.blocked_by_short"}} -
+
{{range .BlockedByDependencies}} -
-
+
+
#{{.Issue.Index}} {{.Issue.Title | ctx.RenderUtils.RenderEmoji}} @@ -63,7 +63,7 @@ {{.Repository.OwnerName}}/{{.Repository.Name}}
-
+
{{if and $.CanCreateIssueDependencies (not $.Repository.IsArchived)}} -
+
+
{{svg "octicon-lock" 16}} @@ -88,7 +88,7 @@ {{.Repository.OwnerName}}/{{.Repository.Name}}
-
+
{{if and $.CanCreateIssueDependencies (not $.Repository.IsArchived)}} -
+
{{if and .CanCreateIssueDependencies (not .Repository.IsArchived)}} - +
{{svg "octicon-trash"}} {{ctx.Locale.Tr "repo.issues.dependency.remove_header"}}
diff --git a/tests/integration/api_issue_dependency_test.go b/tests/integration/api_issue_dependency_test.go index 45ffcd0ee4..5685c217ba 100644 --- a/tests/integration/api_issue_dependency_test.go +++ b/tests/integration/api_issue_dependency_test.go @@ -19,6 +19,7 @@ import ( "gitea.dev/models/unittest" user_model "gitea.dev/models/user" api "gitea.dev/modules/structs" + "gitea.dev/modules/test" repo_service "gitea.dev/services/repository" "gitea.dev/tests" @@ -175,12 +176,9 @@ func TestWebDeleteIssueDependencyCrossRepoPermission(t *testing.T) { "removeDependencyID": strconv.FormatInt(dependencyIssue.ID, 10), "dependencyType": "blockedBy", }) - session.MakeRequest(t, req, http.StatusSeeOther) - - unittest.AssertExistsAndLoadBean(t, &issues_model.IssueDependency{ - IssueID: targetIssue.ID, - DependencyID: dependencyIssue.ID, - }) + resp := session.MakeRequest(t, req, http.StatusBadRequest) + assert.Equal(t, "Permission denied.", test.ParseJSONError(resp.Body.Bytes()).ErrorMessage) + unittest.AssertExistsAndLoadBean(t, &issues_model.IssueDependency{IssueID: targetIssue.ID, DependencyID: dependencyIssue.ID}) assert.NoError(t, repo_service.AddOrUpdateCollaborator(t.Context(), dependencyRepo, user40, perm.AccessModeRead)) @@ -188,10 +186,6 @@ func TestWebDeleteIssueDependencyCrossRepoPermission(t *testing.T) { "removeDependencyID": strconv.FormatInt(dependencyIssue.ID, 10), "dependencyType": "blockedBy", }) - session.MakeRequest(t, req, http.StatusSeeOther) - - unittest.AssertNotExistsBean(t, &issues_model.IssueDependency{ - IssueID: targetIssue.ID, - DependencyID: dependencyIssue.ID, - }) + session.MakeRequest(t, req, http.StatusOK) + unittest.AssertNotExistsBean(t, &issues_model.IssueDependency{IssueID: targetIssue.ID, DependencyID: dependencyIssue.ID}) } diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 0b6007e0e2..ac1e808ff0 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -649,6 +649,13 @@ td .commit-summary { text-decoration: line-through; } +.repository.view.issue .ui.depending .item .item-left { + display: flex; + flex: 1; + flex-direction: column; + min-width: 0; +} + .repository .comment.form .content .field:first-child { clear: none; }