Merge pull request #37 from maykinmedia/feature/33-configurable-reset…

…-db-save-after

✨ [#33] Add reset_db_save_after option to config model

docs/quickstart.rst

@@ -20,7 +20,7 @@ Installation
 
         pip install django-log-outgoing-requests
 
-#.  Add ``log_outgoing_requests`` to ``INSTALLED_APPS`` in your Django 
+#.  Add ``log_outgoing_requests`` to ``INSTALLED_APPS`` in your Django
     project's ``settings.py``.
 
 #. Run ``python manage.py migrate`` to create the necessary database tables
@@ -97,9 +97,10 @@ you likely want to apply the following non-default settings:
     From a security and privacy perspective, we advise not enabling saving to the
     database by default via Django settings and instead rely on runtime configuration.
 
-    If Celery is installed but not configured in your environment, ``LOG_OUTGOING_REQUESTS_RESET_DB_SAVE_AFTER`` 
+    If Celery is installed but not configured in your environment, ``LOG_OUTGOING_REQUESTS_RESET_DB_SAVE_AFTER``
     (which defines if/when database logging is reset after changes to the library config) should
-    be set to ``None``.
+    be set to ``None``. The duration for **Reset saving logs in database after** can also be
+    configured from the admin and will override the value of the environment variable if defined.
 
     The library provides a Django management command as well as a Celery task to delete
     logs which are older than a specified time (by default, 1 day).

log_outgoing_requests/admin.py

@@ -9,6 +9,11 @@
 from .conf import settings
 from .models import OutgoingRequestsLog, OutgoingRequestsLogConfig
 
+try:
+    import celery
+except ImportError:
+    celery = None
+
 
 @admin.register(OutgoingRequestsLog)
 class OutgoingRequestsLogAdmin(admin.ModelAdmin):
@@ -128,3 +133,9 @@ class Meta:
 @admin.register(OutgoingRequestsLogConfig)
 class OutgoingRequestsLogConfigAdmin(SingletonModelAdmin):
     form = ConfigAdminForm
+
+    def get_fields(self, request, obj=None, *args, **kwargs):
+        fields = super().get_fields(request, obj=obj, *args, **kwargs)
+        if celery is None and (obj and not obj.reset_db_save_after):
+            fields.remove("reset_db_save_after")
+        return fields

log_outgoing_requests/config_reset.py

@@ -1,11 +1,13 @@
 # NOTE: since this is imported in models.py, ensure it doesn't use functionality that
 # requires django to be fully initialized.
+from typing import Optional
+
 from .conf import settings
 from .tasks import reset_config
 
 
-def schedule_config_reset():
-    reset_after = settings.LOG_OUTGOING_REQUESTS_RESET_DB_SAVE_AFTER
+def schedule_config_reset(reset_after: Optional[int]):
+    reset_after = reset_after or settings.LOG_OUTGOING_REQUESTS_RESET_DB_SAVE_AFTER
     if not reset_after:
         return
 

log_outgoing_requests/migrations/0006_outgoingrequestslogconfig_reset_db_save_after.py

@@ -0,0 +1,25 @@
+# Generated by Django 3.2.24 on 2024-02-27 09:32
+
+import django.core.validators
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("log_outgoing_requests", "0005_alter_outgoingrequestslog_url"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="outgoingrequestslogconfig",
+            name="reset_db_save_after",
+            field=models.PositiveIntegerField(
+                blank=True,
+                help_text="If configured, after the config has been updated, reset the database logging after the specified number of minutes. Note: this overrides the LOG_OUTGOING_REQUESTS_RESET_DB_SAVE_AFTER environment variable. Additionally, depending on the broker that is used, if this duration is too long the key for the reset task might have expired before that time. So make sure not to set too large a value for the reset.",
+                null=True,
+                validators=[django.core.validators.MinValueValidator(1)],
+                verbose_name="Reset saving logs in database after",
+            ),
+        ),
+    ]

log_outgoing_requests/models.py

@@ -206,13 +206,27 @@ class OutgoingRequestsLogConfig(SingletonModel):
             "If 'Require content length' is not checked, this setting has no effect."
         ),
     )
+    reset_db_save_after = models.PositiveIntegerField(
+        _("Reset saving logs in database after"),
+        null=True,
+        blank=True,
+        validators=[MinValueValidator(1)],
+        help_text=_(
+            "If configured, after the config has been updated, reset the database logging "
+            "after the specified number of minutes. Note: this overrides the "
+            "LOG_OUTGOING_REQUESTS_RESET_DB_SAVE_AFTER environment variable. Additionally, "
+            "depending on the broker that is used, if this duration is too long "
+            "the key for the reset task might have expired before that time. So make sure not to "
+            "set too large a value for the reset."
+        ),
+    )
 
     class Meta:
         verbose_name = _("Outgoing request log configuration")
 
     def save(self, *args, **kwargs):
         super().save(*args, **kwargs)
-        schedule_config_reset()
+        schedule_config_reset(self.reset_db_save_after)
 
     @property
     def save_logs_enabled(self):

tests/test_tasks.py

@@ -94,10 +94,40 @@ def test_saving_config_schedules_config_reset(mocker):
 
 
 @pytest.mark.skipif(not has_celery, reason="Celery is optional dependency")
+@pytest.mark.django_db
 def test_schedule_config_schedules_celery_task(settings, mocker):
+    settings.LOG_OUTGOING_REQUESTS_RESET_DB_SAVE_AFTER = 1
+    config = OutgoingRequestsLogConfig.get_solo()
     mock_task = mocker.patch(
         "log_outgoing_requests.config_reset.reset_config.apply_async"
     )
-    settings.LOG_OUTGOING_REQUESTS_RESET_DB_SAVE_AFTER = 1
-    schedule_config_reset()
+    schedule_config_reset(config.reset_db_save_after)
     mock_task.assert_called_once_with(countdown=60)
+
+
+@pytest.mark.skipif(not has_celery, reason="Celery is optional dependency")
+@pytest.mark.django_db
+def test_schedule_config_schedules_celery_task_use_db_value(settings, mocker):
+    settings.LOG_OUTGOING_REQUESTS_RESET_DB_SAVE_AFTER = 1
+    config = OutgoingRequestsLogConfig.get_solo()
+    config.reset_db_save_after = 2
+    mock_task = mocker.patch(
+        "log_outgoing_requests.config_reset.reset_config.apply_async"
+    )
+    schedule_config_reset(config.reset_db_save_after)
+    mock_task.assert_called_once_with(countdown=120)
+
+
+@pytest.mark.skipif(not has_celery, reason="Celery is optional dependency")
+@pytest.mark.django_db
+def test_schedule_config_schedules_celery_task_after_save_use_db_value(
+    settings, mocker
+):
+    settings.LOG_OUTGOING_REQUESTS_RESET_DB_SAVE_AFTER = 1
+    config = OutgoingRequestsLogConfig.get_solo()
+    mock_task = mocker.patch(
+        "log_outgoing_requests.config_reset.reset_config.apply_async"
+    )
+    config.reset_db_save_after = 4
+    config.save()
+    mock_task.assert_called_once_with(countdown=240)