From 40028868ec68debea4a678048d1930679c777b92 Mon Sep 17 00:00:00 2001 From: Payam Date: Tue, 28 Apr 2026 13:14:30 +0400 Subject: [PATCH] Update version --- django_email_learning/jobs/api/urls.py | 6 + django_email_learning/jobs/api/views.py | 45 +++++++ django_email_learning/jobs/check_imap_job.py | 13 +- .../deactivate_inactive_enrollments_job.py | 14 +-- .../jobs/deliver_contents_job.py | 13 +- .../jobs/send_reminders_job.py | 13 +- .../commands/cleanup_job_executions.py | 53 +++++++++ .../0015_jobexecution_running_uniqueness.py | 49 ++++++++ ...16_alter_jobexecution_job_name_and_more.py | 33 ++++++ django_email_learning/models.py | 27 ++++- django_service/admin.py | 8 ++ pyproject.toml | 2 +- .../test_cleanup_job_executions_view.py | 111 ++++++++++++++++++ 13 files changed, 346 insertions(+), 41 deletions(-) create mode 100644 django_email_learning/management/commands/cleanup_job_executions.py create mode 100644 django_email_learning/migrations/0015_jobexecution_running_uniqueness.py create mode 100644 django_email_learning/migrations/0016_alter_jobexecution_job_name_and_more.py create mode 100644 tests/jobs/api/test_views/test_cleanup_job_executions_view.py diff --git a/django_email_learning/jobs/api/urls.py b/django_email_learning/jobs/api/urls.py index d18d149..7d781da 100644 --- a/django_email_learning/jobs/api/urls.py +++ b/django_email_learning/jobs/api/urls.py @@ -4,6 +4,7 @@ CheckIMAPJobView, SendQuizRemindersJobView, DeactivateInactiveEnrollmentsJobView, + CleanupJobExecutionsView, ) app_name = "django_email_learning" @@ -29,4 +30,9 @@ DeactivateInactiveEnrollmentsJobView.as_view(), name="deactivate_inactive_enrollments", ), + path( + "cleanup_job_executions/", + CleanupJobExecutionsView.as_view(), + name="cleanup_job_executions", + ), ] diff --git a/django_email_learning/jobs/api/views.py b/django_email_learning/jobs/api/views.py index 43eba3e..2522259 100644 --- a/django_email_learning/jobs/api/views.py +++ b/django_email_learning/jobs/api/views.py @@ -8,6 +8,8 @@ ) from django.utils.decorators import method_decorator from django.http import JsonResponse +from django.core.management import call_command +from io import StringIO from django_email_learning.models import JobName from django_email_learning.services.metrics_service import MetricsService @@ -78,3 +80,46 @@ def get(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-unty }, status=500, ) + + +@method_decorator(check_api_key(), name="get") +class CleanupJobExecutionsView(View): + def get(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] + try: + days = request.GET.get("days") + dry_run = request.GET.get("dry_run", "false").lower() in { + "1", + "true", + "yes", + } + + command_stdout = StringIO() + command_kwargs = { + "dry_run": dry_run, + "stdout": command_stdout, + } + if days is not None: + command_kwargs["days"] = int(days) + + call_command("cleanup_job_executions", **command_kwargs) + return JsonResponse( + { + "status": "CleanupJobExecutions command triggered", + "output": command_stdout.getvalue().strip(), + }, + status=202, + ) + except ValueError: + return JsonResponse( + {"status": "CleanupJobExecutions failed", "error": "Invalid days"}, + status=400, + ) + except Exception as e: + metric_service.job_execution_failed(job_name="cleanup_job_executions") + return JsonResponse( + { + "status": "CleanupJobExecutions failed", + "error": str(e), + }, + status=500, + ) diff --git a/django_email_learning/jobs/check_imap_job.py b/django_email_learning/jobs/check_imap_job.py index 03bcde2..c965386 100644 --- a/django_email_learning/jobs/check_imap_job.py +++ b/django_email_learning/jobs/check_imap_job.py @@ -42,19 +42,14 @@ def _get_imap_interface(self) -> ImapInterfaceProtocol: return ImapInterface() def run(self) -> None: - if JobExecution.objects.filter( - job_name=JobName.CHECK_IMAP.value, - status=JobStatus.RUNNING.value, - ).exists(): + job_execution = JobExecution.start_if_not_running( + job_name=JobName.CHECK_IMAP.value + ) + if job_execution is None: logger.warning( "Another instance of CheckIMAPJob is already running. Exiting this run." ) return - job_execution = JobExecution.objects.create( - job_name=JobName.CHECK_IMAP.value, - status=JobStatus.RUNNING.value, - started_at=timezone.now(), - ) self._run_job(job_execution) @track_job_execution( diff --git a/django_email_learning/jobs/deactivate_inactive_enrollments_job.py b/django_email_learning/jobs/deactivate_inactive_enrollments_job.py index 9546376..07279b8 100644 --- a/django_email_learning/jobs/deactivate_inactive_enrollments_job.py +++ b/django_email_learning/jobs/deactivate_inactive_enrollments_job.py @@ -24,20 +24,14 @@ class DeactivateInactiveEnrollmentsJob: def run(self) -> None: - if JobExecution.objects.filter( - job_name=JobName.DEACTIVATE_ENROLLMENTS.value, - status=JobStatus.RUNNING.value, - ).exists(): + job_execution = JobExecution.start_if_not_running( + job_name=JobName.DEACTIVATE_ENROLLMENTS.value + ) + if job_execution is None: logger.warning( "Another instance of DEACTIVATE_ENROLLMENTS is already running. Exiting this run." ) return - - job_execution = JobExecution.objects.create( - job_name=JobName.DEACTIVATE_ENROLLMENTS.value, - status=JobStatus.RUNNING.value, - started_at=timezone.now(), - ) self._run_job(job_execution) @track_job_execution( diff --git a/django_email_learning/jobs/deliver_contents_job.py b/django_email_learning/jobs/deliver_contents_job.py index 9e6dc0f..3676000 100644 --- a/django_email_learning/jobs/deliver_contents_job.py +++ b/django_email_learning/jobs/deliver_contents_job.py @@ -27,19 +27,14 @@ def __init__(self) -> None: self.delivery_queue: DeliveryQueueProtocol = self.get_delivery_queue() def run(self) -> None: - if JobExecution.objects.filter( - job_name=JobName.DELIVER_CONTENTS.value, - status=JobStatus.RUNNING.value, - ).exists(): + job_execution = JobExecution.start_if_not_running( + job_name=JobName.DELIVER_CONTENTS.value + ) + if job_execution is None: logger.warning( "Another instance of DeliverContentsJob is already running. Exiting this run." ) return - job_execution = JobExecution.objects.create( - job_name=JobName.DELIVER_CONTENTS.value, - status=JobStatus.RUNNING.value, - started_at=timezone.now(), - ) self._run_job(job_execution) @track_job_execution( diff --git a/django_email_learning/jobs/send_reminders_job.py b/django_email_learning/jobs/send_reminders_job.py index 3a2bd71..c3d8ac7 100644 --- a/django_email_learning/jobs/send_reminders_job.py +++ b/django_email_learning/jobs/send_reminders_job.py @@ -23,19 +23,14 @@ def __init__(self) -> None: self.reminder_queue: DeliveryQueueProtocol = self.get_reminder_queue() def run(self) -> None: - if JobExecution.objects.filter( - job_name=JobName.SEND_REMINDERS.value, - status=JobStatus.RUNNING.value, - ).exists(): + job_execution = JobExecution.start_if_not_running( + job_name=JobName.SEND_REMINDERS.value + ) + if job_execution is None: logger.warning( "Another instance of SendRemindersJob is already running. Exiting this run." ) return - job_execution = JobExecution.objects.create( - job_name=JobName.SEND_REMINDERS.value, - status=JobStatus.RUNNING.value, - started_at=timezone.now(), - ) self._run_job(job_execution) @track_job_execution( diff --git a/django_email_learning/management/commands/cleanup_job_executions.py b/django_email_learning/management/commands/cleanup_job_executions.py new file mode 100644 index 0000000..5c7c9e8 --- /dev/null +++ b/django_email_learning/management/commands/cleanup_job_executions.py @@ -0,0 +1,53 @@ +from datetime import timedelta + +from django.core.management.base import BaseCommand, CommandParser +from django.utils import timezone + +from django_email_learning.models import JobExecution, JobStatus + + +class Command(BaseCommand): + help = "Delete old completed JobExecution rows to limit table growth" + + def add_arguments(self, parser: CommandParser) -> None: + parser.add_argument( + "--days", + type=int, + default=2, + help="Delete completed job executions finished more than this many days ago", + ) + parser.add_argument( + "--dry-run", + action="store_true", + help="Report how many rows would be deleted without deleting them", + ) + + def handle(self, *args, **options) -> None: # type: ignore[no-untyped-def] + days = options["days"] + dry_run = options["dry_run"] + + if days <= 0: + self.stdout.write(self.style.ERROR("--days must be a positive integer")) + return + + cutoff = timezone.now() - timedelta(days=days) + queryset = JobExecution.objects.filter( + status=JobStatus.COMPLETED.value, + finished_at__isnull=False, + finished_at__lt=cutoff, + ) + + candidate_count = queryset.count() + + if dry_run: + self.stdout.write( + f"Dry run: {candidate_count} completed job executions older than {days} days would be deleted." + ) + return + + deleted_count, _ = queryset.delete() + self.stdout.write( + self.style.SUCCESS( + f"Deleted {deleted_count} completed job executions older than {days} days." + ) + ) diff --git a/django_email_learning/migrations/0015_jobexecution_running_uniqueness.py b/django_email_learning/migrations/0015_jobexecution_running_uniqueness.py new file mode 100644 index 0000000..fccf82e --- /dev/null +++ b/django_email_learning/migrations/0015_jobexecution_running_uniqueness.py @@ -0,0 +1,49 @@ +# Generated by Django 6.0.4 on 2026-04-28 + +from django.db import migrations, models +from django.utils import timezone + + +def deduplicate_running_jobexecutions(apps, schema_editor): # type: ignore[no-untyped-def] + JobExecution = apps.get_model("django_email_learning", "JobExecution") + + running_rows = JobExecution.objects.filter(status="running").order_by( + "job_name", "-started_at", "-id" + ) + + latest_ids_by_job = {} + duplicate_ids = [] + + for row in running_rows: + if row.job_name not in latest_ids_by_job: + latest_ids_by_job[row.job_name] = row.id + continue + duplicate_ids.append(row.id) + + if duplicate_ids: + now = timezone.now() + JobExecution.objects.filter(id__in=duplicate_ids).update( + status="completed", + finished_at=now, + ) + + +class Migration(migrations.Migration): + dependencies = [ + ("django_email_learning", "0014_learner_photo_alter_jobexecution_job_name"), + ] + + operations = [ + migrations.RunPython( + deduplicate_running_jobexecutions, + migrations.RunPython.noop, + ), + migrations.AddConstraint( + model_name="jobexecution", + constraint=models.UniqueConstraint( + condition=models.Q(status="running"), + fields=("job_name",), + name="unique_running_jobexecution_per_job", + ), + ), + ] diff --git a/django_email_learning/migrations/0016_alter_jobexecution_job_name_and_more.py b/django_email_learning/migrations/0016_alter_jobexecution_job_name_and_more.py new file mode 100644 index 0000000..7fed61f --- /dev/null +++ b/django_email_learning/migrations/0016_alter_jobexecution_job_name_and_more.py @@ -0,0 +1,33 @@ +# Generated by Django 6.0.3 on 2026-04-28 18:34 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("django_email_learning", "0015_jobexecution_running_uniqueness"), + ] + + operations = [ + migrations.AlterField( + model_name="jobexecution", + name="job_name", + field=models.CharField( + choices=[ + ("check_imap", "CHECK_IMAP"), + ("deliver_contents", "DELIVER_CONTENTS"), + ("send_reminders", "SEND_REMINDERS"), + ("deactivate_enrollments", "DEACTIVATE_ENROLLMENTS"), + ], + max_length=200, + ), + ), + migrations.AlterField( + model_name="jobexecution", + name="status", + field=models.CharField( + choices=[("running", "RUNNING"), ("completed", "COMPLETED")], + max_length=50, + ), + ), + ] diff --git a/django_email_learning/models.py b/django_email_learning/models.py index 502766e..5344933 100644 --- a/django_email_learning/models.py +++ b/django_email_learning/models.py @@ -10,7 +10,7 @@ from django.conf.global_settings import LANGUAGES from django.core.files.storage import default_storage from django.urls import reverse -from django.db import models, transaction +from django.db import models, transaction, IntegrityError from django.core.validators import ( MaxValueValidator, MinValueValidator, @@ -1028,15 +1028,36 @@ class JobStatus(StrEnum): class JobExecution(models.Model): job_name = models.CharField( - max_length=200, choices=[(job.name, job.value) for job in JobName] + max_length=200, choices=[(job.value, job.name) for job in JobName] ) started_at = models.DateTimeField(auto_now_add=True) finished_at = models.DateTimeField(null=True, blank=True) status = models.CharField( max_length=50, - choices=[(status.name, status.value) for status in JobStatus], + choices=[(status.value, status.name) for status in JobStatus], ) + class Meta: + constraints = [ + models.UniqueConstraint( + fields=["job_name"], + condition=models.Q(status=JobStatus.RUNNING.value), + name="unique_running_jobexecution_per_job", + ) + ] + + @classmethod + def start_if_not_running(cls, job_name: str) -> "JobExecution | None": + try: + with transaction.atomic(): + return cls.objects.create( + job_name=job_name, + status=JobStatus.RUNNING.value, + started_at=timezone.now(), + ) + except IntegrityError: + return None + def __str__(self) -> str: return ( f"Job: {self.job_name} started at {self.started_at} - Status: {self.status}" diff --git a/django_service/admin.py b/django_service/admin.py index 7155e92..1e01231 100644 --- a/django_service/admin.py +++ b/django_service/admin.py @@ -12,6 +12,7 @@ Learner, DeliverySchedule, QuizSubmission, + JobExecution, ) from django_email_learning.oauth_integrations.models import Session @@ -51,6 +52,13 @@ class SessionAdmin(admin.ModelAdmin): list_filter = ("state", "created_at") +class JobExecutionAdmin(admin.ModelAdmin): + list_display = ("id", "job_name", "status", "started_at") + search_fields = ("job_name", "status") + list_filter = ("job_name", "status", "started_at") + + +admin.site.register(JobExecution, JobExecutionAdmin) admin.site.register(Course, CourseAdmin) admin.site.register(ImapConnection, ImapConnectionAdmin) admin.site.register(OrganizationUser) diff --git a/pyproject.toml b/pyproject.toml index 146883e..09ed45d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "django-email-learning" -version = "0.3.6" +version = "0.3.7" description = "A platform for creating and delivering learning materials via email within a Django application. It provides tools for content management, user role-based administration, and scheduler integration for automated content delivery." authors = [ {name = "Payam Najafizadeh",email = "payam.nj@gmail.com"} diff --git a/tests/jobs/api/test_views/test_cleanup_job_executions_view.py b/tests/jobs/api/test_views/test_cleanup_job_executions_view.py new file mode 100644 index 0000000..dcd0619 --- /dev/null +++ b/tests/jobs/api/test_views/test_cleanup_job_executions_view.py @@ -0,0 +1,111 @@ +from unittest import mock + +import pytest +from django.urls import reverse + +from django_email_learning.services.jwt_service import generate_jwt + +URL = reverse("django_email_learning:api_jobs:cleanup_job_executions") + + +def test_cleanup_job_executions_without_api_key(superadmin_client): + response = superadmin_client.get(URL) + assert response.status_code == 401 + assert response.json() == {"error": "Authorization header missing"} + + +@pytest.mark.parametrize("api_key", ["Basic valid_api_key", "without_space"]) +def test_cleanup_job_executions_with_invalid_api_key(superadmin_client, api_key): + response = superadmin_client.get( + URL, + HTTP_AUTHORIZATION=api_key, + ) + assert response.status_code == 401 + assert response.json() == { + "error": "Invalid Authorization header format. Expected: Bearer " + } + + +def test_cleanup_job_executions_with_invalid_decoded_api_key(superadmin_client): + response = superadmin_client.get( + URL, + HTTP_AUTHORIZATION="Bearer invalid_decoded_api_key", + ) + assert response.status_code == 401 + assert response.json() == {"error": "Invalid Json Web Token"} + + +def test_cleanup_job_executions_valid_jwt_format_but_invalid_api_key(superadmin_client): + jwt = generate_jwt({"key": "invalid_key", "salt": "salt"}) + response = superadmin_client.get( + URL, + HTTP_AUTHORIZATION=f"Bearer {jwt}", + ) + assert response.status_code == 401 + assert response.json() == {"error": "Invalid API key"} + + +@mock.patch( + "django_email_learning.jobs.api.views.call_command", + return_value=None, +) +def test_cleanup_job_executions_with_valid_api_key( + mock_call_command, superadmin_client +): + create_key_response = superadmin_client.post( + reverse("django_email_learning:api_platform:api_key_view") + ) + response = superadmin_client.get( + URL, + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + ) + + assert response.status_code == 202 + assert response.json() == { + "status": "CleanupJobExecutions command triggered", + "output": "", + } + mock_call_command.assert_called_once() + + +@mock.patch( + "django_email_learning.jobs.api.views.call_command", + side_effect=Exception("boom"), +) +@mock.patch( + "django_email_learning.jobs.api.views.metric_service.job_execution_failed", +) +def test_cleanup_job_executions_failed_triggers_job_execution_failed_metric( + mock_job_execution_failed, mock_call_command, superadmin_client +): + create_key_response = superadmin_client.post( + reverse("django_email_learning:api_platform:api_key_view") + ) + response = superadmin_client.get( + URL, + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + ) + + assert response.status_code == 500 + assert response.json() == { + "status": "CleanupJobExecutions failed", + "error": "boom", + } + mock_call_command.assert_called_once() + mock_job_execution_failed.assert_called_once_with(job_name="cleanup_job_executions") + + +def test_cleanup_job_executions_returns_400_for_invalid_days(superadmin_client): + create_key_response = superadmin_client.post( + reverse("django_email_learning:api_platform:api_key_view") + ) + response = superadmin_client.get( + f"{URL}?days=invalid", + HTTP_AUTHORIZATION=f"Bearer {create_key_response.json()['key']}", + ) + + assert response.status_code == 400 + assert response.json() == { + "status": "CleanupJobExecutions failed", + "error": "Invalid days", + }