[fix] Fixed timezone handling in delete_old_radiusbatch_users command #611

- Fixed date/datetime comparison issue in delete_old_radiusbatch_users command
- Added freezegun to test for triggering bug on purpose
- Loop over objects with iterator() in delete_old_radiusbatch_users in delete_old_radiusbatch_users

Closes #611

---------

Co-authored-by: Federico Capoano <f.capoano@openwisp.io>

Author: Eeshu-Yadav

openwisp_radius/management/commands/base/delete_old_radiusbatch_users.py

@@ -33,11 +33,9 @@ def handle(self, *args, **options):
             days = 30 * options["older_than_months"]
         else:
             days = BATCH_DELETE_EXPIRED
-        threshold_date = now() - timedelta(days=days)
-
+        threshold_date = (now() - timedelta(days=days)).date()
         batches = RadiusBatch.objects.filter(expiration_date__lt=threshold_date)
-        time_period = threshold_date.strftime("%Y-%m-%d %H:%M:%S")
-
-        for b in batches:
+        for b in batches.iterator():
             b.delete()
+        time_period = threshold_date.strftime("%Y-%m-%d %H:%M:%S")
         self.stdout.write(f"Deleted accounts older than {time_period}")

openwisp_radius/tests/test_commands.py

@@ -6,14 +6,15 @@
 from django.contrib.auth import get_user_model
 from django.core.management import CommandError, call_command
 from django.utils.timezone import get_default_timezone, now
+from freezegun import freeze_time
 from netaddr import EUI, mac_unix
 from openvpn_status.models import Routing
 
 from openwisp_utils.tests import capture_any_output, capture_stdout
 
 from .. import settings as app_settings
 from ..utils import load_model
-from . import _RADACCT, CallCommandMixin, FileMixin
+from . import _RADACCT, _TEST_DATE, CallCommandMixin, FileMixin
 from .mixins import BaseTestCase
 
 User = get_user_model()
@@ -164,8 +165,9 @@ def test_deactivate_expired_users_command(self):
         call_command("deactivate_expired_users")
         self.assertEqual(get_user_model().objects.filter(is_active=True).count(), 0)
 
+    @freeze_time(_TEST_DATE)
     @capture_stdout()
-    def test_delete_old_radiusbatch_users_command(self):
+    def test_delete_old_radiusbatch_users_command(self, subtest_id=None):
         # Create RadiusBatch users that expired more than 18 months ago
         path = self._get_path("static/test_batch.csv")
         options = dict(
@@ -214,9 +216,11 @@ def test_delete_old_radiusbatch_users_command(self):
 
         with self.subTest("Test executing command with both arguments"):
             options["name"] = "test3"
-            call_command("batch_add_users", **options)
+            self._call_command("batch_add_users", **options)
             call_command(
-                "delete_old_radiusbatch_users", older_than_days=9, older_than_months=12
+                "delete_old_radiusbatch_users",
+                older_than_days=9,
+                older_than_months=12,
             )
             # Users that expired more than 9 days ago should be deleted
             self.assertEqual(get_user_model().objects.all().count(), 0)