Merge pull request '[BUG] Don't delete inactive emails explicitly' (#2880) from gusted/forgejo-inactive-email into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2880
Reviewed-by: Otto <otto@codeberg.org>

models/user/email_address.go

@@ -278,14 +278,6 @@ func IsEmailUsed(ctx context.Context, email string) (bool, error) {
 	return db.GetEngine(ctx).Where("lower_email=?", strings.ToLower(email)).Get(&EmailAddress{})
 }
 
-// DeleteInactiveEmailAddresses deletes inactive email addresses
-func DeleteInactiveEmailAddresses(ctx context.Context) error {
-	_, err := db.GetEngine(ctx).
-		Where("is_activated = ?", false).
-		Delete(new(EmailAddress))
-	return err
-}
-
 // ActivateEmail activates the email address to given user.
 func ActivateEmail(ctx context.Context, email *EmailAddress) error {
 	ctx, committer, err := db.TxContext(ctx)

services/user/user.go

@@ -304,5 +304,5 @@ func DeleteInactiveUsers(ctx context.Context, olderThan time.Duration) error {
 		}
 	}
 
-	return user_model.DeleteInactiveEmailAddresses(ctx)
+	return nil
 }

services/user/user_test.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"strings"
 	"testing"
+	"time"
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/models/auth"
@@ -16,6 +17,7 @@ import (
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/timeutil"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -184,3 +186,33 @@ func TestCreateUser_Issue5882(t *testing.T) {
 		assert.NoError(t, DeleteUser(db.DefaultContext, v.user, false))
 	}
 }
+
+func TestDeleteInactiveUsers(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+	// Add an inactive user older than a minute, with an associated email_address record.
+	oldUser := &user_model.User{Name: "OldInactive", LowerName: "oldinactive", Email: "old@example.com", CreatedUnix: timeutil.TimeStampNow().Add(-120)}
+	_, err := db.GetEngine(db.DefaultContext).NoAutoTime().Insert(oldUser)
+	assert.NoError(t, err)
+	oldEmail := &user_model.EmailAddress{UID: oldUser.ID, IsPrimary: true, Email: "old@example.com", LowerEmail: "old@example.com"}
+	err = db.Insert(db.DefaultContext, oldEmail)
+	assert.NoError(t, err)
+
+	// Add an inactive user that's not older than a minute, with an associated email_address record.
+	newUser := &user_model.User{Name: "NewInactive", LowerName: "newinactive", Email: "new@example.com"}
+	err = db.Insert(db.DefaultContext, newUser)
+	assert.NoError(t, err)
+	newEmail := &user_model.EmailAddress{UID: newUser.ID, IsPrimary: true, Email: "new@example.com", LowerEmail: "new@example.com"}
+	err = db.Insert(db.DefaultContext, newEmail)
+	assert.NoError(t, err)
+
+	err = DeleteInactiveUsers(db.DefaultContext, time.Minute)
+	assert.NoError(t, err)
+
+	// User older than a minute should be deleted along with their email address.
+	unittest.AssertExistsIf(t, false, oldUser)
+	unittest.AssertExistsIf(t, false, oldEmail)
+
+	// User not older than a minute shouldn't be deleted and their emaill address should still exist.
+	unittest.AssertExistsIf(t, true, newUser)
+	unittest.AssertExistsIf(t, true, newEmail)
+}