Skip to content

Commit 99b5634

Browse files
lunnylunny
committed
Fix
1 parent 1ecac1a commit 99b5634

File tree

4 files changed

+128
-132
lines changed

4 files changed

+128
-132
lines changed

‎services/doctor/review.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2021 The Gitea Authors. All rights reserved.
1+
// Copyright 2025 The Gitea Authors. All rights reserved.
22
// SPDX-License-Identifier: MIT
33

44
package doctor

‎services/issue/comments.go‎

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -134,35 +134,33 @@ func UpdateComment(ctx context.Context, c *issues_model.Comment, contentVersion
134134

135135
// DeleteComment deletes the comment
136136
func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) error {
137-
err := db.WithTx(ctx, func(ctx context.Context) error {
138-
if err := issues_model.DeleteComment(ctx, comment); err != nil {
137+
if comment.ReviewID > 0 {
138+
if err := comment.LoadIssue(ctx); err != nil {
139139
return err
140140
}
141-
142-
if comment.ReviewID > 0 {
143-
if err := comment.LoadIssue(ctx); err != nil {
144-
return err
145-
}
146-
if err := comment.Issue.LoadRepo(ctx); err != nil {
147-
return err
148-
}
149-
if err := comment.Issue.LoadPullRequest(ctx); err != nil {
150-
return err
151-
}
152-
if err := git.RemoveRef(ctx, comment.Issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(comment.Issue.PullRequest.Index, comment.ID)); err != nil {
153-
log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", comment.Issue.PullRequest.ID, err)
154-
// We should not return error here, because the comment has been removed from database.
155-
// users have to delete this ref manually or we should have a synchronize between
156-
// database comment table and git refs.
157-
}
141+
if err := comment.Issue.LoadRepo(ctx); err != nil {
142+
return err
158143
}
144+
if err := comment.Issue.LoadPullRequest(ctx); err != nil {
145+
return err
146+
}
147+
}
159148

160-
return nil
161-
})
162-
if err != nil {
149+
if err := db.WithTx(ctx, func(ctx context.Context) error {
150+
return issues_model.DeleteComment(ctx, comment)
151+
}); err != nil {
163152
return err
164153
}
165154

155+
if comment.ReviewID > 0 {
156+
if err := git.RemoveRef(ctx, comment.Issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(comment.Issue.PullRequest.Index, comment.ID)); err != nil {
157+
log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", comment.Issue.PullRequest.ID, err)
158+
// We should not return error here, because the comment has been removed from database.
159+
// users have to delete this ref manually or we should have a synchronize between
160+
// database comment table and git refs.
161+
}
162+
}
163+
166164
notify_service.DeleteComment(ctx, doer, comment)
167165

168166
return nil

‎services/issue/issue.go‎

Lines changed: 104 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ func DeleteIssue(ctx context.Context, doer *user_model.User, gitRepo *git.Reposi
191191
}
192192

193193
// delete entries in database
194-
attachmentPaths, err := deleteIssue(ctx, issue)
194+
attachmentPaths, comments, err := deleteIssue(ctx, issue)
195195
if err != nil {
196196
return err
197197
}
@@ -211,6 +211,17 @@ func DeleteIssue(ctx context.Context, doer *user_model.User, gitRepo *git.Reposi
211211
}
212212
}
213213

214+
for _, comment := range comments {
215+
if comment.ReviewID > 0 {
216+
if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID)); err != nil {
217+
log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", issue.PullRequest.ID, err)
218+
// We should not return error here, because the comment has been removed from database.
219+
// users have to delete this ref manually or we should have a synchronize between
220+
// database comment table and git refs.
221+
}
222+
}
223+
}
224+
214225
notify_service.DeleteIssue(ctx, doer, issue)
215226

216227
return nil
@@ -266,131 +277,107 @@ func GetRefEndNamesAndURLs(issues []*issues_model.Issue, repoLink string) (map[i
266277
}
267278

268279
// deleteIssue deletes the issue
269-
func deleteIssue(ctx context.Context, issue *issues_model.Issue) ([]string, error) {
270-
ctx, committer, err := db.TxContext(ctx)
271-
if err != nil {
272-
return nil, err
273-
}
274-
defer committer.Close()
275-
276-
// update the total issue numbers
277-
if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil {
278-
return nil, err
279-
}
280-
// if the issue is closed, update the closed issue numbers
281-
if issue.IsClosed {
282-
if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil {
283-
return nil, err
280+
func deleteIssue(ctx context.Context, issue *issues_model.Issue) ([]string, []*issues_model.Comment, error) {
281+
var attachmentPaths []string
282+
if err := db.WithTx(ctx, func(ctx context.Context) error {
283+
// update the total issue numbers
284+
if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil {
285+
return err
286+
}
287+
// if the issue is closed, update the closed issue numbers
288+
if issue.IsClosed {
289+
if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil {
290+
return err
291+
}
284292
}
285-
}
286-
287-
if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil {
288-
return nil, fmt.Errorf("error updating counters for milestone id %d: %w",
289-
issue.MilestoneID, err)
290-
}
291293

292-
if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID, issue.Index); err != nil {
293-
return nil, err
294-
}
294+
if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil {
295+
return fmt.Errorf("error updating counters for milestone id %d: %w",
296+
issue.MilestoneID, err)
297+
}
295298

296-
// find attachments related to this issue and remove them
297-
if err := issue.LoadAttachments(ctx); err != nil {
298-
return nil, err
299-
}
299+
if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID, issue.Index); err != nil {
300+
return err
301+
}
300302

301-
var attachmentPaths []string
302-
for i := range issue.Attachments {
303-
attachmentPaths = append(attachmentPaths, issue.Attachments[i].RelativePath())
304-
}
303+
// find attachments related to this issue and remove them
304+
if err := issue.LoadAttachments(ctx); err != nil {
305+
return err
306+
}
305307

306-
// deference all review comments
307-
if err := issue.LoadRepo(ctx); err != nil {
308-
return nil, err
309-
}
310-
if err := issue.LoadPullRequest(ctx); err != nil {
311-
return nil, err
312-
}
308+
for i := range issue.Attachments {
309+
attachmentPaths = append(attachmentPaths, issue.Attachments[i].RelativePath())
310+
}
313311

314-
if err := issue.LoadComments(ctx); err != nil {
315-
return nil, err
316-
}
312+
// deference all review comments
313+
if err := issue.LoadRepo(ctx); err != nil {
314+
return err
315+
}
316+
if err := issue.LoadPullRequest(ctx); err != nil {
317+
return err
318+
}
317319

318-
// delete all database data still assigned to this issue
319-
if err := db.DeleteBeans(ctx,
320-
&issues_model.ContentHistory{IssueID: issue.ID},
321-
&issues_model.IssueLabel{IssueID: issue.ID},
322-
&issues_model.IssueDependency{IssueID: issue.ID},
323-
&issues_model.IssueAssignees{IssueID: issue.ID},
324-
&issues_model.IssueUser{IssueID: issue.ID},
325-
&activities_model.Notification{IssueID: issue.ID},
326-
&issues_model.Reaction{IssueID: issue.ID},
327-
&issues_model.IssueWatch{IssueID: issue.ID},
328-
&issues_model.Stopwatch{IssueID: issue.ID},
329-
&issues_model.TrackedTime{IssueID: issue.ID},
330-
&project_model.ProjectIssue{IssueID: issue.ID},
331-
&repo_model.Attachment{IssueID: issue.ID},
332-
&issues_model.PullRequest{IssueID: issue.ID},
333-
&issues_model.Comment{RefIssueID: issue.ID},
334-
&issues_model.IssueDependency{DependencyID: issue.ID},
335-
&issues_model.Comment{DependentIssueID: issue.ID},
336-
&issues_model.IssuePin{IssueID: issue.ID},
337-
); err != nil {
338-
return nil, err
339-
}
320+
if err := issue.LoadComments(ctx); err != nil {
321+
return err
322+
}
340323

341-
for _, comment := range issue.Comments {
342-
if err := issues_model.DeleteComment(ctx, comment); err != nil {
343-
return nil, err
324+
// delete all database data still assigned to this issue
325+
if err := db.DeleteBeans(ctx,
326+
&issues_model.ContentHistory{IssueID: issue.ID},
327+
&issues_model.IssueLabel{IssueID: issue.ID},
328+
&issues_model.IssueDependency{IssueID: issue.ID},
329+
&issues_model.IssueAssignees{IssueID: issue.ID},
330+
&issues_model.IssueUser{IssueID: issue.ID},
331+
&activities_model.Notification{IssueID: issue.ID},
332+
&issues_model.Reaction{IssueID: issue.ID},
333+
&issues_model.IssueWatch{IssueID: issue.ID},
334+
&issues_model.Stopwatch{IssueID: issue.ID},
335+
&issues_model.TrackedTime{IssueID: issue.ID},
336+
&project_model.ProjectIssue{IssueID: issue.ID},
337+
&repo_model.Attachment{IssueID: issue.ID},
338+
&issues_model.PullRequest{IssueID: issue.ID},
339+
&issues_model.Comment{RefIssueID: issue.ID},
340+
&issues_model.IssueDependency{DependencyID: issue.ID},
341+
&issues_model.Comment{DependentIssueID: issue.ID},
342+
&issues_model.IssuePin{IssueID: issue.ID},
343+
); err != nil {
344+
return err
344345
}
345346

346-
if comment.ReviewID > 0 {
347-
if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID)); err != nil {
348-
log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", issue.PullRequest.ID, err)
349-
// We should not return error here, because the comment has been removed from database.
350-
// users have to delete this ref manually or we should have a synchronize between
351-
// database comment table and git refs.
347+
for _, comment := range issue.Comments {
348+
if err := issues_model.DeleteComment(ctx, comment); err != nil {
349+
return err
352350
}
353351
}
354352

355-
// delete all attachments related to this comment
356-
for _, attachment := range comment.Attachments {
357-
if err := storage.Attachments.Delete(repo_model.AttachmentRelativePath(attachment.UUID)); err != nil {
358-
// Even delete files failed, but the attachments has been removed from database, so we
359-
// should not return error but only record the error on logs.
360-
// users have to delete this attachments manually or we should have a
361-
// synchronize between database attachment table and attachment storage
362-
log.Error("delete attachment[uuid: %s] failed: %v", attachment.UUID, err)
353+
// delete all pull request records
354+
if issue.IsPull {
355+
// Delete scheduled auto merges
356+
if _, err := db.GetEngine(ctx).Where("pull_id=?", issue.PullRequest.ID).
357+
Delete(&pull_model.AutoMerge{}); err != nil {
358+
return err
363359
}
364-
}
365-
}
366360

367-
// delete all pull request records
368-
if issue.IsPull {
369-
// Delete scheduled auto merges
370-
if _, err := db.GetEngine(ctx).Where("pull_id=?", issue.PullRequest.ID).
371-
Delete(&pull_model.AutoMerge{}); err != nil {
372-
return nil, err
373-
}
361+
// Delete review states
362+
if _, err := db.GetEngine(ctx).Where("pull_id=?", issue.PullRequest.ID).
363+
Delete(&pull_model.ReviewState{}); err != nil {
364+
return err
365+
}
374366

375-
// Delete review states
376-
if _, err := db.GetEngine(ctx).Where("pull_id=?", issue.PullRequest.ID).
377-
Delete(&pull_model.ReviewState{}); err != nil {
378-
return nil, err
367+
if _, err := db.GetEngine(ctx).ID(issue.PullRequest.ID).Delete(&issues_model.PullRequest{}); err != nil {
368+
return err
369+
}
379370
}
380371

381-
if _, err := db.GetEngine(ctx).ID(issue.PullRequest.ID).Delete(&issues_model.PullRequest{}); err != nil {
382-
return nil, err
372+
if _, err := db.GetEngine(ctx).ID(issue.ID).NoAutoCondition().Delete(issue); err != nil {
373+
return err
383374
}
375+
return nil
376+
}); err != nil {
377+
return nil, nil, err
384378
}
385379

386-
if _, err := db.GetEngine(ctx).ID(issue.ID).NoAutoCondition().Delete(issue); err != nil {
387-
return nil, err
388-
}
389-
390-
if err := committer.Commit(); err != nil {
391-
return nil, err
392-
}
393-
return attachmentPaths, nil
380+
return attachmentPaths, issue.Comments, nil
394381
}
395382

396383
// DeleteOrphanedIssues delete issues without a repo
@@ -438,11 +425,22 @@ func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths []
438425
}
439426

440427
for _, issue := range issues {
441-
issueAttachPaths, err := deleteIssue(ctx, issue)
428+
issueAttachPaths, comments, err := deleteIssue(ctx, issue)
442429
if err != nil {
443430
return nil, fmt.Errorf("deleteIssue [issue_id: %d]: %w", issue.ID, err)
444431
}
445432

433+
for _, comment := range comments {
434+
if comment.ReviewID > 0 {
435+
if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID)); err != nil {
436+
log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", issue.PullRequest.ID, err)
437+
// We should not return error here, because the comment has been removed from database.
438+
// users have to delete this ref manually or we should have a synchronize between
439+
// database comment table and git refs.
440+
}
441+
}
442+
}
443+
446444
attachmentPaths = append(attachmentPaths, issueAttachPaths...)
447445
}
448446
}

‎services/issue/issue_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func TestIssue_DeleteIssue(t *testing.T) {
4141

4242
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: issueIDs[2]})
4343

44-
_, err = deleteIssue(db.DefaultContext, issue)
44+
_, _, err = deleteIssue(db.DefaultContext, issue)
4545
assert.NoError(t, err)
4646
issueIDs, err = issues_model.GetIssueIDsByRepoID(db.DefaultContext, 1)
4747
assert.NoError(t, err)
@@ -52,7 +52,7 @@ func TestIssue_DeleteIssue(t *testing.T) {
5252
assert.NoError(t, err)
5353
issue, err = issues_model.GetIssueByID(db.DefaultContext, 4)
5454
assert.NoError(t, err)
55-
_, err = deleteIssue(db.DefaultContext, issue)
55+
_, _, err = deleteIssue(db.DefaultContext, issue)
5656
assert.NoError(t, err)
5757
assert.Len(t, attachments, 2)
5858
for i := range attachments {
@@ -75,7 +75,7 @@ func TestIssue_DeleteIssue(t *testing.T) {
7575
assert.NoError(t, err)
7676
assert.False(t, left)
7777

78-
_, err = deleteIssue(db.DefaultContext, issue2)
78+
_, _, err = deleteIssue(db.DefaultContext, issue2)
7979
assert.NoError(t, err)
8080
left, err = issues_model.IssueNoDependenciesLeft(db.DefaultContext, issue1)
8181
assert.NoError(t, err)

0 commit comments

Comments
 (0)