Repository: openwisp/openwisp-radius (language: python)

Issue:
[feature] Add management command and celery task to delete unverified users
This was a feature that was previously used in OpenWISP1, certain type of unverified users were removed automatically.

We should replicate the same, following something similar to what we did previously, see:
https://github.com/openwisp/openwisp-radius/blob/211c30cd1d249acb73619c2a39a77a2f6324d500/openwisp_radius/tasks.py

The new command shall look for users which have a RegisteredUser relation and RegisteredUser.is_verified=False and have been created for more than X days ago, where days is is a parameter defaulting to 1 day (24 hours).

The command shall accept an `exclude_methods` parameter which can contain a list of verification methods to exclude from the query, so that certain verification methods can be excluded and not deleted anyway.

The command shall be documented as the other ones.
[feature] Add management command and celery task to delete unverified users
This was a feature that was previously used in OpenWISP1, certain type of unverified users were removed automatically.

We should replicate the same, following something similar to what we did previously, see:
https://github.com/openwisp/openwisp-radius/blob/211c30cd1d249acb73619c2a39a77a2f6324d500/openwisp_radius/tasks.py

The new command shall look for users which have a RegisteredUser relation and RegisteredUser.is_verified=False and have been created for more than X days ago, where days is is a parameter defaulting to 1 day (24 hours).

The command shall accept an `exclude_methods` parameter which can contain a list of verification methods to exclude from the query, so that certain verification methods can be excluded and not deleted anyway.

The command shall be documented as the other ones.


Discussion / hints:
> The command shall accept an exclude_methods parameter which can contain a list of verification methods to exclude from the query, so that certain verification methods can be excluded and not deleted anyway.

Do you mean registration method here? 
> The command shall accept an exclude_methods parameter which can contain a list of verification methods to exclude from the query, so that certain verification methods can be excluded and not deleted anyway.

Do you mean registration method here? 

Fix patch:
diff --git a/docs/source/user/management_commands.rst b/docs/source/user/management_commands.rst
index 1f9e6b4d..5e4a6517 100644
--- a/docs/source/user/management_commands.rst
+++ b/docs/source/user/management_commands.rst
@@ -85,6 +85,25 @@ This command deletes users that have expired (and should have been deactivated b
 
 Note that the default duration is set to 18 months.
 
+``delete_unverified_users``
+---------------------------
+
+This command deletes unverified users that have been registered for
+more than specified duration.
+
+.. code-block:: shell
+
+    ./manage.py delete_unverified_users --older-than-days <duration_in_days>
+
+Note that the default duration is set to 1 day.
+
+It is also possible to exclude users that have registered using specified methods.
+You can specify multiple methods separated by comma(`,`). Following is an example:
+
+.. code-block:: shell
+
+    ./manage.py delete_unverified_users --older-than-days 1 --exclude-methods mobile_phone,email
+
 ``upgrade_from_django_freeradius``
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/openwisp_radius/management/commands/base/delete_unverified_users.py b/openwisp_radius/management/commands/base/delete_unverified_users.py
new file mode 100644
index 00000000..45c62e95
--- /dev/null
+++ b/openwisp_radius/management/commands/base/delete_unverified_users.py
@@ -0,0 +1,39 @@
+from datetime import timedelta
+
+from django.contrib.auth import get_user_model
+from django.core.management import BaseCommand
+from django.utils.timezone import now
+
+User = get_user_model()
+
+
+class BaseDeleteUnverifiedUsersCommand(BaseCommand):
+    help = 'Delete unverified users older than <days>'
+
+    def add_arguments(self, parser):
+        parser.add_argument(
+            '--older-than-days',
+            action='store',
+            default=1,
+            help='delete unverified users which registered days before this',
+        )
+        parser.add_argument(
+            '--exclude-methods',
+            action='store',
+            default='',
+            help='list of registration methods to skip',
+        )
+
+    def handle(self, *args, **options):
+        days = now() - timedelta(days=int(options['older_than_days']))
+        exclude_methods = str(options['exclude_methods']).split(',')
+        User.objects.filter(
+            date_joined__lt=days,
+            registered_user__isnull=False,
+            registered_user__is_verified=False,
+        ).exclude(registered_user__method__in=exclude_methods).delete()
+        self.stdout.write(
+            'Deleted unverified accounts older than '
+            f'{options["older_than_days"]} day(s) with registration method '
+            f'other than {options["exclude_methods"]}'
+        )
diff --git a/openwisp_radius/management/commands/delete_unverified_users.py b/openwisp_radius/management/commands/delete_unverified_users.py
new file mode 100644
index 00000000..288a0221
--- /dev/null
+++ b/openwisp_radius/management/commands/delete_unverified_users.py
@@ -0,0 +1,5 @@
+from .base.delete_unverified_users import BaseDeleteUnverifiedUsersCommand
+
+
+class Command(BaseDeleteUnverifiedUsersCommand):
+    pass
diff --git a/openwisp_radius/tasks.py b/openwisp_radius/tasks.py
index 41a40028..c4dcda89 100644
--- a/openwisp_radius/tasks.py
+++ b/openwisp_radius/tasks.py
@@ -25,3 +25,12 @@ def deactivate_expired_users():
 @shared_task
 def delete_old_users(older_than_months=12):
     management.call_command('delete_old_users', older_than_months=older_than_months)
+
+
+@shared_task
+def delete_unverified_users(older_than_days=1, exclude_methods=''):
+    management.call_command(
+        'delete_unverified_users',
+        older_than_days=older_than_days,
+        exclude_methods=exclude_methods,
+    )


Test patch:
diff --git a/openwisp_radius/tests/test_commands.py b/openwisp_radius/tests/test_commands.py
index dfe6c254..d6ecfcbf 100644
--- a/openwisp_radius/tests/test_commands.py
+++ b/openwisp_radius/tests/test_commands.py
@@ -16,6 +16,7 @@
 RadiusAccounting = load_model('RadiusAccounting')
 RadiusBatch = load_model('RadiusBatch')
 RadiusPostAuth = load_model('RadiusPostAuth')
+RegisteredUser = load_model('RegisteredUser')
 
 
 class TestCommands(FileMixin, CallCommandMixin, BaseTestCase):
@@ -157,3 +158,59 @@ def test_prefix_add_users_command(self):
         )
         with self.assertRaises(SystemExit):
             self._call_command('prefix_add_users', **options)
+
+    @capture_stdout()
+    def test_unverified_users_command(self):
+        def _create_old_users():
+            User.objects.all().delete()
+            RadiusBatch.objects.all().delete()
+            path = self._get_path('static/test_batch.csv')
+            options = dict(organization=self.default_org.slug, file=path, name='test',)
+            self._call_command('batch_add_users', **options)
+            User.objects.update(date_joined=now() - timedelta(days=3))
+            for user in User.objects.all():
+                RegisteredUser.objects.create(
+                    user=user, method='email', is_verified=False
+                )
+
+        with self.subTest('Delete unverified users older than 2 days'):
+            _create_old_users()
+            # This user should not be deleted
+            RegisteredUser.objects.create(
+                user=self._create_user(), method='mobile_phone', is_verified=False
+            )
+
+            self.assertEqual(User.objects.count(), 4)
+            call_command('delete_unverified_users', older_than_days=2)
+            self.assertEqual(User.objects.count(), 1)
+
+        with self.subTest(
+            'Delete unverified users except registered with mobile_phone'
+        ):
+            _create_old_users()
+            # This user should not be deleted
+            RegisteredUser.objects.create(
+                user=self._create_user(date_joined=now() - timedelta(days=3)),
+                method='mobile_phone',
+                is_verified=False,
+            )
+
+            self.assertEqual(User.objects.count(), 4)
+            call_command(
+                'delete_unverified_users',
+                older_than_days=2,
+                exclude_methods='mobile_phone,manual',
+            )
+            self.assertEqual(User.objects.count(), 1)
+
+        with self.subTest('Verified user should not be deleted'):
+            _create_old_users()
+            # This user should not be deleted
+            RegisteredUser.objects.create(
+                user=self._create_user(date_joined=now() - timedelta(days=3)),
+                method='email',
+                is_verified=True,
+            )
+            self.assertEqual(User.objects.count(), 4)
+            call_command('delete_unverified_users',)
+            self.assertEqual(User.objects.count(), 1)
diff --git a/openwisp_radius/tests/test_tasks.py b/openwisp_radius/tests/test_tasks.py
index 625b4c2e..7d3a0ae3 100644
--- a/openwisp_radius/tests/test_tasks.py
+++ b/openwisp_radius/tests/test_tasks.py
@@ -1,6 +1,10 @@
+from datetime import timedelta
+
 from celery import Celery
 from celery.contrib.testing.worker import start_worker
 from django.contrib.auth import get_user_model
+from django.core import management
+from django.utils.timezone import now
 
 from openwisp_radius import tasks
 from openwisp_utils.tests import capture_any_output, capture_stdout
@@ -13,6 +17,7 @@
 RadiusAccounting = load_model('RadiusAccounting')
 RadiusBatch = load_model('RadiusBatch')
 RadiusPostAuth = load_model('RadiusPostAuth')
+RegisteredUser = load_model('RegisteredUser')
 
 
 class TestCelery(FileMixin, BaseTestCase):
@@ -83,3 +88,15 @@ def test_delete_old_radacct(self):
         result = tasks.delete_old_radacct.delay(3)
         self.assertTrue(result.successful())
         self.assertEqual(RadiusAccounting.objects.filter(unique_id='666').count(), 0)
+
+    @capture_stdout()
+    def test_delete_unverified_users(self):
+        path = self._get_path('static/test_batch.csv')
+        options = dict(organization=self.default_org.slug, file=path, name='test',)
+        management.call_command('batch_add_users', **options)
+        User.objects.update(date_joined=now() - timedelta(days=3))
+        for user in User.objects.all():
+            RegisteredUser.objects.create(user=user, method='email', is_verified=False)
+        self.assertEqual(User.objects.count(), 3)
+        tasks.delete_unverified_users.delay(older_than_days=2)
+        self.assertEqual(User.objects.count(), 0)
diff --git a/tests/openwisp2/sample_radius/management/commands/delete_unverified_users.py b/tests/openwisp2/sample_radius/management/commands/delete_unverified_users.py
new file mode 100644
index 00000000..3a888ac3
--- /dev/null
+++ b/tests/openwisp2/sample_radius/management/commands/delete_unverified_users.py
@@ -0,0 +1,7 @@
+from openwisp_radius.management.commands.base.delete_unverified_users import (
+    BaseDeleteUnverifiedUsersCommand,
+)
+
+
+class Command(BaseDeleteUnverifiedUsersCommand):
+    pass