From 9bf821ae6c108379d22ae11d8d5784a4ed7ad647 Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Wed, 20 Nov 2024 20:55:32 -0800
Subject: [PATCH] Fix GetInactiveUsers (#32540)

Fix #31480
---
 models/fixtures/user.yml |  1 +
 models/user/user.go      | 18 ++++++++++++------
 models/user/user_test.go | 14 ++++++++++++++
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml
index d7316e9b388..b3bece5589c 100644
--- a/models/fixtures/user.yml
+++ b/models/fixtures/user.yml
@@ -333,6 +333,7 @@
   repo_admin_change_team_access: false
   theme: ""
   keep_activity_private: false
+  created_unix: 1730468968
 
 -
   id: 10
diff --git a/models/user/user.go b/models/user/user.go
index c1cb988e43d..a2d91662919 100644
--- a/models/user/user.go
+++ b/models/user/user.go
@@ -48,19 +48,19 @@ const (
 	UserTypeIndividual UserType = iota // Historic reason to make it starts at 0.
 
 	// UserTypeOrganization defines an organization
-	UserTypeOrganization
+	UserTypeOrganization // 1
 
 	// UserTypeUserReserved reserves a (non-existing) user, i.e. to prevent a spam user from re-registering after being deleted, or to reserve the name until the user is actually created later on
-	UserTypeUserReserved
+	UserTypeUserReserved // 2
 
 	// UserTypeOrganizationReserved reserves a (non-existing) organization, to be used in combination with UserTypeUserReserved
-	UserTypeOrganizationReserved
+	UserTypeOrganizationReserved // 3
 
 	// UserTypeBot defines a bot user
-	UserTypeBot
+	UserTypeBot // 4
 
 	// UserTypeRemoteUser defines a remote user for federated users
-	UserTypeRemoteUser
+	UserTypeRemoteUser // 5
 )
 
 const (
@@ -884,7 +884,13 @@ func UpdateUserCols(ctx context.Context, u *User, cols ...string) error {
 
 // GetInactiveUsers gets all inactive users
 func GetInactiveUsers(ctx context.Context, olderThan time.Duration) ([]*User, error) {
-	var cond builder.Cond = builder.Eq{"is_active": false}
+	cond := builder.And(
+		builder.Eq{"is_active": false},
+		builder.Or( // only plain user
+			builder.Eq{"`type`": UserTypeIndividual},
+			builder.Eq{"`type`": UserTypeUserReserved},
+		),
+	)
 
 	if olderThan > 0 {
 		cond = cond.And(builder.Lt{"created_unix": time.Now().Add(-olderThan).Unix()})
diff --git a/models/user/user_test.go b/models/user/user_test.go
index bc1abc64512..6701be39a55 100644
--- a/models/user/user_test.go
+++ b/models/user/user_test.go
@@ -588,3 +588,17 @@ func TestDisabledUserFeatures(t *testing.T) {
 		assert.True(t, user_model.IsFeatureDisabledWithLoginType(user, f))
 	}
 }
+
+func TestGetInactiveUsers(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	// all inactive users
+	// user1's createdunix is 1730468968
+	users, err := user_model.GetInactiveUsers(db.DefaultContext, 0)
+	assert.NoError(t, err)
+	assert.Len(t, users, 1)
+	interval := time.Now().Unix() - 1730468968 + 3600*24
+	users, err = user_model.GetInactiveUsers(db.DefaultContext, time.Duration(interval*int64(time.Second)))
+	assert.NoError(t, err)
+	assert.Len(t, users, 0)
+}