feature: simplifies inactive user filtering; adds tests

rhodecode/model/db.py

@@ -726,7 +726,6 @@ class User(Base, BaseModel):
 
         cutoff_date = datetime.datetime.now(datetime.timezone.utc) - datetime.timedelta(days=older_then_days)
 
-        never_logged_in = ((cls.last_login == None) & (cls.last_activity == None) & (cls.created_on < cutoff_date))
         last_activity_long_ago_or_no_activity_since_creation = (
             (cls.last_activity < cutoff_date) | ((cls.last_activity == None) & (cls.created_on < cutoff_date))
         )
@@ -736,10 +735,7 @@ class User(Base, BaseModel):
 
         query = query.filter(
             exclude_system_users,
-            or_(
+            last_activity_long_ago_or_no_activity_since_creation
-                never_logged_in,
-                last_activity_long_ago_or_no_activity_since_creation
-            )
         )
 
         return query.all()

rhodecode/tests/lib/rc_commands/test_inactive_users.py

@@ -0,0 +1,65 @@
+from datetime import datetime, timedelta
+
+import pytest
+
+from rhodecode.lib.rc_commands.inactive_users import find_inactive_users
+from rhodecode.model.db import User
+
+
+@pytest.mark.usefixtures("app", "user_util")
+class TestFilterInactiveUsers(object):
+    def test_filter_inactive_users__user_with_email_never_logged_in(self, user_util):
+        inactive_user = user_util.create_user(username="user_to_be_removed")
+        user_util.create_user(username="normal_user")
+
+        created_long_ago = datetime.now() - timedelta(days=300)
+        inactive_user.created_on = created_long_ago
+        user_id = inactive_user.user_id
+
+        inactive_users_with_email, inactive_users_without_email = find_inactive_users()
+
+        assert len(inactive_users_without_email) == 0
+        assert len(inactive_users_with_email) == 1
+        assert inactive_users_with_email[0].user_id == user_id
+
+    def test_filter_inactive_users__user_without_email_never_logged_in(self, user_util):
+        inactive_user = user_util.create_user(username="user_to_be_removed", email=None)
+        user_util.create_user(username="normal_user")
+
+        created_long_ago = datetime.now() - timedelta(days=300)
+        inactive_user.created_on = created_long_ago
+        user_id = inactive_user.user_id
+
+        inactive_users_with_email, inactive_users_without_email = find_inactive_users()
+
+        assert len(inactive_users_without_email) == 1
+        assert len(inactive_users_with_email) == 0
+        assert inactive_users_without_email[0].user_id == user_id
+
+    def test_filter_inactive_users__user_with_email_last_activity_long_ago(self, user_util):
+        inactive_user = user_util.create_user(username="user_to_be_removed")
+        user_util.create_user(username="normal_user")
+
+        activity_long_ago = datetime.now() - timedelta(days=300)
+        inactive_user.last_activity = activity_long_ago
+        user_id = inactive_user.user_id
+
+        inactive_users_with_email, inactive_users_without_email = find_inactive_users()
+
+        assert len(inactive_users_without_email) == 0
+        assert len(inactive_users_with_email) == 1
+        assert inactive_users_with_email[0].user_id == user_id
+
+    def test_filter_inactive_users__user_without_email_last_activity_long_ago(self, user_util):
+        inactive_user = user_util.create_user(username="user_to_be_removed", email=None)
+        user_util.create_user(username="normal_user")
+
+        activity_long_ago = datetime.now() - timedelta(days=300)
+        inactive_user.last_activity = activity_long_ago
+        user_id = inactive_user.user_id
+
+        inactive_users_with_email, inactive_users_without_email = find_inactive_users()
+
+        assert len(inactive_users_without_email) == 1
+        assert len(inactive_users_with_email) == 0
+        assert inactive_users_without_email[0].user_id == user_id