From 920f6d24d28d2c044eb3ed10e6a281a4523b9fbb Mon Sep 17 00:00:00 2001 From: floss4good Date: Sun, 29 Jun 2025 12:08:03 +0200 Subject: [PATCH] fix: load OldMilestone based on OldMilestoneID, not MilestoneID (#8330) Fixes #8329 ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. ### Documentation - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [x] I want the title to show in the release notes with a link to this pull request. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8330 Reviewed-by: Robert Wolff Co-authored-by: floss4good Co-committed-by: floss4good --- models/fixtures/comment.yml | 38 ++++++++++++++++++++++++- models/issues/comment_list.go | 16 +++++------ tests/integration/issue_comment_test.go | 26 ++++++++++++++++- 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/models/fixtures/comment.yml b/models/fixtures/comment.yml index 34407d6f81..6908d85dda 100644 --- a/models/fixtures/comment.yml +++ b/models/fixtures/comment.yml @@ -186,10 +186,46 @@ type: 8 # milestone poster_id: 1 issue_id: 1 # in repo_id 1 - milestone_id: 10 # not exsting milestone + milestone_id: 10 # not existing milestone old_milestone_id: 0 created_unix: 946685080 +- + id: 2004 + type: 8 # milestone + poster_id: 1 + issue_id: 1 # in repo_id 1 + milestone_id: 1 + old_milestone_id: 10 # not existing (ghost) milestone + created_unix: 946685085 + +- + id: 2005 + type: 8 # milestone + poster_id: 1 + issue_id: 1 # in repo_id 1 + milestone_id: 10 # not existing (ghost) milestone + old_milestone_id: 1 + created_unix: 946685090 + +- + id: 2006 + type: 8 # milestone + poster_id: 1 + issue_id: 1 # in repo_id 1 + milestone_id: 11 # not existing (ghost) milestone + old_milestone_id: 10 # not existing (ghost) milestone + created_unix: 946685095 + +- + id: 2007 + type: 8 # milestone + poster_id: 1 + issue_id: 1 # in repo_id 1 + milestone_id: 0 + old_milestone_id: 11 # not existing (ghost) milestone + created_unix: 946685100 + - id: 2010 type: 30 # project diff --git a/models/issues/comment_list.go b/models/issues/comment_list.go index 7285e347b4..9b502d1c91 100644 --- a/models/issues/comment_list.go +++ b/models/issues/comment_list.go @@ -101,7 +101,7 @@ func (comments CommentList) loadMilestones(ctx context.Context) error { return nil } - milestoneMaps := make(map[int64]*Milestone, len(milestoneIDs)) + milestones := make(map[int64]*Milestone, len(milestoneIDs)) left := len(milestoneIDs) for left > 0 { limit := db.DefaultMaxInSize @@ -110,7 +110,7 @@ func (comments CommentList) loadMilestones(ctx context.Context) error { } err := db.GetEngine(ctx). In("id", milestoneIDs[:limit]). - Find(&milestoneMaps) + Find(&milestones) if err != nil { return err } @@ -118,8 +118,8 @@ func (comments CommentList) loadMilestones(ctx context.Context) error { milestoneIDs = milestoneIDs[limit:] } - for _, issue := range comments { - issue.Milestone = milestoneMaps[issue.MilestoneID] + for _, comment := range comments { + comment.Milestone = milestones[comment.MilestoneID] } return nil } @@ -140,7 +140,7 @@ func (comments CommentList) loadOldMilestones(ctx context.Context) error { return nil } - milestoneMaps := make(map[int64]*Milestone, len(milestoneIDs)) + milestones := make(map[int64]*Milestone, len(milestoneIDs)) left := len(milestoneIDs) for left > 0 { limit := db.DefaultMaxInSize @@ -149,7 +149,7 @@ func (comments CommentList) loadOldMilestones(ctx context.Context) error { } err := db.GetEngine(ctx). In("id", milestoneIDs[:limit]). - Find(&milestoneMaps) + Find(&milestones) if err != nil { return err } @@ -157,8 +157,8 @@ func (comments CommentList) loadOldMilestones(ctx context.Context) error { milestoneIDs = milestoneIDs[limit:] } - for _, issue := range comments { - issue.OldMilestone = milestoneMaps[issue.MilestoneID] + for _, comment := range comments { + comment.OldMilestone = milestones[comment.OldMilestoneID] } return nil } diff --git a/tests/integration/issue_comment_test.go b/tests/integration/issue_comment_test.go index f77bfaa9bd..0c53c3028b 100644 --- a/tests/integration/issue_comment_test.go +++ b/tests/integration/issue_comment_test.go @@ -102,11 +102,35 @@ func TestIssueCommentChangeMilestone(t *testing.T) { []string{"user1 removed this from the milestone2 milestone"}, []string{"/user1", "/user2/repo1/milestone/2"}) - // Deleted milestone + // Added milestone that in the meantime was deleted testIssueCommentChangeEvent(t, htmlDoc, "2003", "octicon-milestone", "User One", "/user1", []string{"user1 added this to the (deleted) milestone"}, []string{"/user1"}) + + // Modified milestone - from a meantime deleted one to a valid one + testIssueCommentChangeEvent(t, htmlDoc, "2004", + "octicon-milestone", "User One", "/user1", + []string{"user1 modified the milestone from (deleted) to milestone1"}, + []string{"/user1", "/user2/repo1/milestone/1"}) + + // Modified milestone - from a valid one to a meantime deleted one + testIssueCommentChangeEvent(t, htmlDoc, "2005", + "octicon-milestone", "User One", "/user1", + []string{"user1 modified the milestone from milestone1 to (deleted)"}, + []string{"/user1", "/user2/repo1/milestone/1"}) + + // Modified milestone - from a meantime deleted one to a meantime deleted one + testIssueCommentChangeEvent(t, htmlDoc, "2006", + "octicon-milestone", "User One", "/user1", + []string{"user1 modified the milestone from (deleted) to (deleted)"}, + []string{"/user1"}) + + // Removed milestone that in the meantime was deleted + testIssueCommentChangeEvent(t, htmlDoc, "2007", + "octicon-milestone", "User One", "/user1", + []string{"user1 removed this from the (deleted) milestone"}, + []string{"/user1"}) } func TestIssueCommentChangeProject(t *testing.T) {