From c7b5791b56dc9cb9170e785754c5b52b67696e6e Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Tue, 15 Apr 2025 12:35:17 -0700 Subject: [PATCH] Enhancement: allow negative workflow trigger offset --- ...er_workflowtrigger_schedule_offset_days.py | 22 ++++++ src/documents/models.py | 2 +- src/documents/tasks.py | 76 +++++++++++++------ src/documents/tests/test_workflows.py | 50 ++++++++++++ 4 files changed, 126 insertions(+), 24 deletions(-) create mode 100644 src/documents/migrations/1066_alter_workflowtrigger_schedule_offset_days.py diff --git a/src/documents/migrations/1066_alter_workflowtrigger_schedule_offset_days.py b/src/documents/migrations/1066_alter_workflowtrigger_schedule_offset_days.py new file mode 100644 index 000000000..eaf23ad64 --- /dev/null +++ b/src/documents/migrations/1066_alter_workflowtrigger_schedule_offset_days.py @@ -0,0 +1,22 @@ +# Generated by Django 5.1.7 on 2025-04-15 19:18 + +from django.db import migrations +from django.db import models + + +class Migration(migrations.Migration): + dependencies = [ + ("documents", "1065_workflowaction_assign_custom_fields_values"), + ] + + operations = [ + migrations.AlterField( + model_name="workflowtrigger", + name="schedule_offset_days", + field=models.IntegerField( + default=0, + help_text="The number of days to offset the schedule trigger by.", + verbose_name="schedule offset days", + ), + ), + ] diff --git a/src/documents/models.py b/src/documents/models.py index 4b3f97e50..74090700c 100644 --- a/src/documents/models.py +++ b/src/documents/models.py @@ -1019,7 +1019,7 @@ class WorkflowTrigger(models.Model): verbose_name=_("has this correspondent"), ) - schedule_offset_days = models.PositiveIntegerField( + schedule_offset_days = models.IntegerField( _("schedule offset days"), default=0, help_text=_( diff --git a/src/documents/tasks.py b/src/documents/tasks.py index 7d71d48c9..2b4877db6 100644 --- a/src/documents/tasks.py +++ b/src/documents/tasks.py @@ -1,8 +1,8 @@ +import datetime import hashlib import logging import shutil import uuid -from datetime import timedelta from pathlib import Path from tempfile import TemporaryDirectory @@ -356,7 +356,7 @@ def empty_trash(doc_ids=None): if doc_ids is not None else Document.deleted_objects.filter( deleted_at__lt=timezone.localtime(timezone.now()) - - timedelta( + - datetime.timedelta( days=settings.EMPTY_TRASH_DELAY, ), ) @@ -396,6 +396,7 @@ def check_scheduled_workflows(): ) if scheduled_workflows.count() > 0: logger.debug(f"Checking {len(scheduled_workflows)} scheduled workflows") + now = timezone.now() for workflow in scheduled_workflows: schedule_triggers = workflow.triggers.filter( type=WorkflowTrigger.WorkflowTriggerType.SCHEDULED, @@ -403,31 +404,60 @@ def check_scheduled_workflows(): trigger: WorkflowTrigger for trigger in schedule_triggers: documents = Document.objects.none() - offset_td = timedelta(days=trigger.schedule_offset_days) + offset_td = datetime.timedelta(days=-trigger.schedule_offset_days) + threshold = now - offset_td logger.debug( - f"Checking trigger {trigger} with offset {offset_td} against field: {trigger.schedule_date_field}", + f"Trigger {trigger.id}: checking if (date + {offset_td}) <= now ({now})", ) + match trigger.schedule_date_field: case WorkflowTrigger.ScheduleDateField.ADDED: - documents = Document.objects.filter( - added__lt=timezone.now() - offset_td, - ) + documents = Document.objects.filter(added__lte=threshold) + case WorkflowTrigger.ScheduleDateField.CREATED: - documents = Document.objects.filter( - created__lt=timezone.now() - offset_td, - ) + documents = Document.objects.filter(created__lte=threshold) + case WorkflowTrigger.ScheduleDateField.MODIFIED: - documents = Document.objects.filter( - modified__lt=timezone.now() - offset_td, - ) + documents = Document.objects.filter(modified__lte=threshold) + case WorkflowTrigger.ScheduleDateField.CUSTOM_FIELD: - cf_instances = CustomFieldInstance.objects.filter( - field=trigger.schedule_date_custom_field, - value_date__lt=timezone.now() - offset_td, - ) - documents = Document.objects.filter( - id__in=cf_instances.values_list("document", flat=True), + # cap earliest date to avoid massive scans + earliest_date = now - datetime.timedelta(days=365) + if offset_td.days < -365: + logger.warning( + f"Trigger {trigger.id} has large negative offset ({offset_td.days}), " + f"limiting earliest scan date to {earliest_date}", + ) + + cf_filter_kwargs = { + "field": trigger.schedule_date_custom_field, + "value_date__isnull": False, + "value_date__lte": threshold, + "value_date__gte": earliest_date, + } + + recent_cf_instances = CustomFieldInstance.objects.filter( + **cf_filter_kwargs, ) + + matched_ids = [ + cfi.document_id + for cfi in recent_cf_instances + if cfi.value_date + and ( + timezone.make_aware( + datetime.datetime.combine( + cfi.value_date, + datetime.time.min, + ), + ) + + offset_td + <= now + ) + ] + + documents = Document.objects.filter(id__in=matched_ids) + if documents.count() > 0: logger.debug( f"Found {documents.count()} documents for trigger {trigger}", @@ -439,18 +469,18 @@ def check_scheduled_workflows(): workflow=workflow, ).order_by("-run_at") if not trigger.schedule_is_recurring and workflow_runs.exists(): - # schedule is non-recurring and the workflow has already been run logger.debug( f"Skipping document {document} for non-recurring workflow {workflow} as it has already been run", ) continue - elif ( + + if ( trigger.schedule_is_recurring and workflow_runs.exists() and ( workflow_runs.last().run_at - > timezone.now() - - timedelta( + > now + - datetime.timedelta( days=trigger.schedule_recurring_interval_days, ) ) diff --git a/src/documents/tests/test_workflows.py b/src/documents/tests/test_workflows.py index 3006594cc..466a852a5 100644 --- a/src/documents/tests/test_workflows.py +++ b/src/documents/tests/test_workflows.py @@ -1600,6 +1600,56 @@ class TestWorkflows( doc.refresh_from_db() self.assertIsNone(doc.owner) + def test_workflow_scheduled_trigger_negative_offset(self): + """ + GIVEN: + - Existing workflow with SCHEDULED trigger and negative offset of -7 days + WHEN: + - Scheduled workflows are checked for document with custom field date 8 days in the past + THEN: + - Workflow runs and document owner is updated + """ + trigger = WorkflowTrigger.objects.create( + type=WorkflowTrigger.WorkflowTriggerType.SCHEDULED, + schedule_offset_days=-7, + schedule_date_field=WorkflowTrigger.ScheduleDateField.CUSTOM_FIELD, + schedule_date_custom_field=self.cf1, + ) + action = WorkflowAction.objects.create( + assign_title="Doc assign owner", + assign_owner=self.user2, + ) + w = Workflow.objects.create( + name="Workflow 1", + order=0, + ) + w.triggers.add(trigger) + w.actions.add(action) + w.save() + + doc = Document.objects.create( + title="sample test", + correspondent=self.c, + original_filename="sample.pdf", + ) + cfi = CustomFieldInstance.objects.create( + document=doc, + field=self.cf1, + value_date=timezone.now() - timedelta(days=5), + ) + + tasks.check_scheduled_workflows() + + doc.refresh_from_db() + self.assertIsNone(doc.owner) # has not triggered yet + + cfi.value_date = timezone.now() - timedelta(days=8) + cfi.save() + + tasks.check_scheduled_workflows() + doc.refresh_from_db() + self.assertEqual(doc.owner, self.user2) + def test_workflow_enabled_disabled(self): trigger = WorkflowTrigger.objects.create( type=WorkflowTrigger.WorkflowTriggerType.DOCUMENT_ADDED,