feat(email): reference the commit closing the issue (#9522)

Preview-on: https://codeberg.org/attachments/0016d106-0353-43d2-953e-4f2ae1a0d166
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9522
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>

.deadcode-out

@@ -224,6 +224,9 @@ forgejo.org/services/context
 forgejo.org/services/federation
 	FollowRemoteActor
 
+forgejo.org/services/mailer
+	ActionCloseIssueByCommit.isActionAdditionalData
+
 forgejo.org/services/notify
 	UnregisterNotifier
 

options/locale_next/locale_en-US.json

@@ -114,6 +114,7 @@
     "mail.actions.run_info_previous_status": "Previous Run's Status: %[1]s",
     "mail.actions.run_info_sha": "Commit: %[1]s",
     "mail.actions.run_info_trigger": "Triggered because: %[1]s by: %[2]s",
+    "mail.issue.action.close_by_commit": "%[1]s closed %[2]s in commit %[3]s.",
     "repo.diff.commit.next-short": "Next",
     "repo.diff.commit.previous-short": "Prev",
     "discussion.locked": "This discussion has been locked. Commenting is limited to contributors.",

services/mailer/mail.go

@@ -292,6 +292,11 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient
 		"Language":        locale.Language(),
 		"CanReply":        setting.IncomingEmail.Enabled && commentType != issues_model.CommentTypePullRequestPush,
 	}
+	if closeIssueByCommit, ok := ctx.ActionAdditionalData.(ActionCloseIssueByCommit); ok {
+		mailMeta["CloseIssueByCommit"] = closeIssueByCommit.CommitID
+	} else {
+		mailMeta["CloseIssueByCommit"] = ""
+	}
 
 	var mailSubject bytes.Buffer
 	if err := subjectTemplates.ExecuteTemplate(&mailSubject, tplName, mailMeta); err == nil {

services/mailer/mail_issue.go

@@ -25,6 +25,16 @@ func fallbackMailSubject(issue *issues_model.Issue) string {
 	return fmt.Sprintf("[%s] %s (Issue #%d)", issue.Repo.FullName(), issue.Title, issue.Index)
 }
 
+type ActionAdditionalData interface {
+	isActionAdditionalData()
+}
+
+type ActionCloseIssueByCommit struct {
+	CommitID string
+}
+
+func (ActionCloseIssueByCommit) isActionAdditionalData() {}
+
 type mailCommentContext struct {
 	context.Context
 	Issue                 *issues_model.Issue
@@ -33,6 +43,7 @@ type mailCommentContext struct {
 	Content               string
 	Comment               *issues_model.Comment
 	ForceDoerNotification bool
+	ActionAdditionalData  ActionAdditionalData
 }
 
 const (
@@ -174,7 +185,7 @@ func mailIssueCommentBatch(ctx *mailCommentContext, users []*user_model.User, vi
 
 // MailParticipants sends new issue thread created emails to repository watchers
 // and mentioned people.
-func MailParticipants(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, opType activities_model.ActionType, mentions []*user_model.User) error {
+func MailParticipants(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, opType activities_model.ActionType, mentions []*user_model.User, additionalData ActionAdditionalData) error {
 	if setting.MailService == nil {
 		// No mail service configured
 		return nil
@@ -196,6 +207,7 @@ func MailParticipants(ctx context.Context, issue *issues_model.Issue, doer *user
 			Content:               content,
 			Comment:               nil,
 			ForceDoerNotification: forceDoerNotification,
+			ActionAdditionalData:  additionalData,
 		}, mentions); err != nil {
 		log.Error("mailIssueCommentToParticipants: %v", err)
 	}

services/mailer/mail_issue_test.go

@@ -0,0 +1,61 @@
+// Copyright 2025 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package mailer_test
+
+import (
+	"testing"
+
+	"forgejo.org/models/db"
+	issues_model "forgejo.org/models/issues"
+	"forgejo.org/models/unittest"
+	user_model "forgejo.org/models/user"
+	issue_service "forgejo.org/services/issue"
+	"forgejo.org/services/mailer"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCloseIssue(t *testing.T) {
+	defer require.NoError(t, unittest.PrepareTestDatabase())
+
+	called := false
+	defer mailer.MockMailSettings(func(msgs ...*mailer.Message) {
+		require.Len(t, msgs, 3)
+		msg := msgs[0]
+		assert.Equal(t, "Re: [user2/repo1] issue1 (Issue #1)", msg.Subject)
+		mailer.AssertTranslatedLocale(t, msg.Body, "mail.issue.action.close")
+		assert.Contains(t, msg.Body, "closed #1.")
+		called = true
+	})()
+
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
+	err := issue_service.ChangeStatus(db.DefaultContext, issue, user, "", true)
+	require.NoError(t, err)
+	assert.True(t, called)
+}
+
+func TestCloseIssueByCommit(t *testing.T) {
+	defer require.NoError(t, unittest.PrepareTestDatabase())
+
+	called := false
+	defer mailer.MockMailSettings(func(msgs ...*mailer.Message) {
+		require.Len(t, msgs, 3)
+		msg := msgs[0]
+		assert.Equal(t, "Re: [user2/repo1] issue1 (Issue #1)", msg.Subject)
+		mailer.AssertTranslatedLocale(t, msg.Body, "mail.issue.action.close_by_commit")
+		assert.Contains(t, msg.Body, "closed")
+		assert.Contains(t, msg.Body, "#1")
+		assert.Contains(t, msg.Body, "in commit")
+		assert.Contains(t, msg.Body, "abc123def")
+		called = true
+	})()
+
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
+	err := issue_service.ChangeStatus(db.DefaultContext, issue, user, "abc123def", true)
+	require.NoError(t, err)
+	assert.True(t, called)
+}

services/mailer/mail_test.go

@@ -239,6 +239,13 @@ func TestMailerIssueTemplate(t *testing.T) {
 		Content: rejectComment.Content, Comment: rejectComment,
 	})
 	expect(t, msg, pull, rejectComment.Content)
+
+	msg = testCompose(t, &mailCommentContext{
+		Issue: issue, Doer: doer, ActionType: activities_model.ActionCloseIssue,
+		Content: comment.Content, Comment: comment,
+		ActionAdditionalData: ActionCloseIssueByCommit{CommitID: "abc123def"},
+	})
+	expect(t, msg, issue, comment.Content, "abc123def")
 }
 
 func TestTemplateSelection(t *testing.T) {

services/mailer/notify.go

@@ -50,13 +50,14 @@ func (m *mailNotifier) CreateIssueComment(ctx context.Context, doer *user_model.
 }
 
 func (m *mailNotifier) NewIssue(ctx context.Context, issue *issues_model.Issue, mentions []*user_model.User) {
-	if err := MailParticipants(ctx, issue, issue.Poster, activities_model.ActionCreateIssue, mentions); err != nil {
+	if err := MailParticipants(ctx, issue, issue.Poster, activities_model.ActionCreateIssue, mentions, nil); err != nil {
 		log.Error("MailParticipants: %v", err)
 	}
 }
 
 func (m *mailNotifier) IssueChangeStatus(ctx context.Context, doer *user_model.User, commitID string, issue *issues_model.Issue, actionComment *issues_model.Comment, isClosed bool) {
 	var actionType activities_model.ActionType
+	var actionAdditionalData ActionAdditionalData
 	if issue.IsPull {
 		if isClosed {
 			actionType = activities_model.ActionClosePullRequest
@@ -66,12 +67,17 @@ func (m *mailNotifier) IssueChangeStatus(ctx context.Context, doer *user_model.U
 	} else {
 		if isClosed {
 			actionType = activities_model.ActionCloseIssue
+			if commitID != "" {
+				// An issue being closed *and* a commitID being present means that the issue was closed by a PR or
+				// commit message that closed it by reference.
+				actionAdditionalData = ActionCloseIssueByCommit{CommitID: commitID}
+			}
 		} else {
 			actionType = activities_model.ActionReopenIssue
 		}
 	}
 
-	if err := MailParticipants(ctx, issue, doer, actionType, nil); err != nil {
+	if err := MailParticipants(ctx, issue, doer, actionType, nil, actionAdditionalData); err != nil {
 		log.Error("MailParticipants: %v", err)
 	}
 }
@@ -82,14 +88,14 @@ func (m *mailNotifier) IssueChangeTitle(ctx context.Context, doer *user_model.Us
 		return
 	}
 	if issue.IsPull && issues_model.HasWorkInProgressPrefix(oldTitle) && !issue.PullRequest.IsWorkInProgress(ctx) {
-		if err := MailParticipants(ctx, issue, doer, activities_model.ActionPullRequestReadyForReview, nil); err != nil {
+		if err := MailParticipants(ctx, issue, doer, activities_model.ActionPullRequestReadyForReview, nil, nil); err != nil {
 			log.Error("MailParticipants: %v", err)
 		}
 	}
 }
 
 func (m *mailNotifier) NewPullRequest(ctx context.Context, pr *issues_model.PullRequest, mentions []*user_model.User) {
-	if err := MailParticipants(ctx, pr.Issue, pr.Issue.Poster, activities_model.ActionCreatePullRequest, mentions); err != nil {
+	if err := MailParticipants(ctx, pr.Issue, pr.Issue.Poster, activities_model.ActionCreatePullRequest, mentions, nil); err != nil {
 		log.Error("MailParticipants: %v", err)
 	}
 }
@@ -139,7 +145,7 @@ func (m *mailNotifier) MergePullRequest(ctx context.Context, doer *user_model.Us
 		log.Error("LoadIssue: %v", err)
 		return
 	}
-	if err := MailParticipants(ctx, pr.Issue, doer, activities_model.ActionMergePullRequest, nil); err != nil {
+	if err := MailParticipants(ctx, pr.Issue, doer, activities_model.ActionMergePullRequest, nil, nil); err != nil {
 		log.Error("MailParticipants: %v", err)
 	}
 }
@@ -149,7 +155,7 @@ func (m *mailNotifier) AutoMergePullRequest(ctx context.Context, doer *user_mode
 		log.Error("pr.LoadIssue: %v", err)
 		return
 	}
-	if err := MailParticipants(ctx, pr.Issue, doer, activities_model.ActionAutoMergePullRequest, nil); err != nil {
+	if err := MailParticipants(ctx, pr.Issue, doer, activities_model.ActionAutoMergePullRequest, nil, nil); err != nil {
 		log.Error("MailParticipants: %v", err)
 	}
 }

templates/mail/issue/default.tmpl

@@ -35,7 +35,16 @@
 	{{end}}
 	<p>
 		{{if eq .ActionName "close"}}
-			{{.locale.Tr "mail.issue.action.close" .Doer.Name .Issue.Index}}
+			{{if ne .CloseIssueByCommit ""}}
+				{{$commitUrl := printf "%s/commit/%s" .Issue.Repo.HTMLURL .CloseIssueByCommit}}
+				{{$shortSha := ShortSha .CloseIssueByCommit}}
+				{{$commitLink := HTMLFormat "<a href='%[1]s'><b>%[2]s</b></a>" $commitUrl $shortSha}}
+				{{$doer := HTMLFormat "<b>@%[1]s</b>" .Doer.Name}}
+				{{$issue := HTMLFormat "<a href='%[1]s'>#%[2]d</a>" .Link .Issue.Index}}
+				{{.locale.Tr "mail.issue.action.close_by_commit" $doer $issue $commitLink}}
+			{{else}}
+				{{.locale.Tr "mail.issue.action.close" .Doer.Name .Issue.Index}}
+			{{end}}
 		{{else if eq .ActionName "reopen"}}
 			{{.locale.Tr "mail.issue.action.reopen" .Doer.Name .Issue.Index}}
 		{{else if eq .ActionName "merge"}}