diff --git a/django_forms_workflows/admin.py b/django_forms_workflows/admin.py index 6b66e98..fcddf49 100644 --- a/django_forms_workflows/admin.py +++ b/django_forms_workflows/admin.py @@ -278,8 +278,6 @@ def _render_conditional_rules(rules) -> str: [SHOW] when ``first_enrollment`` equals ``Yes`` [REQUIRE] when ``first_enrollment`` equals ``Yes`` """ - import json - if not rules: return "" if isinstance(rules, str): @@ -1279,8 +1277,6 @@ def sync_pull_admin_view(self, request): remote_forms = payload.get("forms", []) # Build per-form diffs: remote vs local - import json - from .diff_views import _build_summary form_diffs = [] diff --git a/django_forms_workflows/email_backends/gmail_api.py b/django_forms_workflows/email_backends/gmail_api.py index fbd5886..6edc74d 100644 --- a/django_forms_workflows/email_backends/gmail_api.py +++ b/django_forms_workflows/email_backends/gmail_api.py @@ -72,6 +72,7 @@ def _is_retryable_gmail_error(exc) -> bool: if err.get("reason") in _RETRYABLE_GMAIL_REASONS: return True except (ValueError, AttributeError, KeyError, TypeError): + # A malformed error payload is not enough evidence to retry safely. pass return False diff --git a/django_forms_workflows/forms.py b/django_forms_workflows/forms.py index f81d7f7..f47a111 100644 --- a/django_forms_workflows/forms.py +++ b/django_forms_workflows/forms.py @@ -341,6 +341,8 @@ def __init__( try: self.stashed_files = json.loads(prev) except (json.JSONDecodeError, TypeError): + # Ignore malformed client metadata; uploaded files are rebuilt + # from validated server-side data below. pass # Build form fields from definition @@ -1177,8 +1179,6 @@ def clean(self): Hidden fields are also dropped from ``cleaned_data`` so that their values are not persisted in the submission. """ - import json - from .conditions import evaluate_conditions cleaned_data = super().clean() @@ -1307,8 +1307,6 @@ def get_enhancements_config(self): Generate JavaScript configuration for form enhancements. Returns a dictionary that can be serialized to JSON. """ - import json - from django.urls import reverse # Disable auto-save for anonymous users (no drafts without a user) @@ -1554,8 +1552,6 @@ def _build_fields(self): def _add_field(self, field_def): """Add a single field to the form.""" - is_editable = True - # Get current value from form data. # Sub-workflow fields are stored with an index suffix (e.g. payment_dept_code_1); # try the indexed key first, then fall back to the bare field name. @@ -1572,16 +1568,16 @@ def _add_field(self, field_def): current_value = field_def.default_value # Auto-fill approver name from current user - if is_editable and self._is_approver_name_field(field_def): + if self._is_approver_name_field(field_def): current_value = self._get_approver_name() # Auto-fill date with current date - elif is_editable and self._is_date_field(field_def): + elif self._is_date_field(field_def): current_value = date.today() # Common field arguments field_args = { "label": field_def.field_label, - "required": field_def.required if is_editable else False, + "required": field_def.required, "help_text": field_def.help_text, "initial": current_value, } @@ -1593,17 +1589,11 @@ def _add_field(self, field_def): if field_def.css_class: widget_attrs["class"] = field_def.css_class - # Make non-editable fields read-only - if not is_editable: - widget_attrs["readonly"] = "readonly" - widget_attrs["disabled"] = "disabled" - field_args["required"] = False - if field_def.field_type in _PM_OPT_OUT_FIELD_TYPES: widget_attrs.update(_PM_OPT_OUT_ATTRS) # Create appropriate field type - self._create_field(field_def, field_args, widget_attrs, is_editable) + self._create_field(field_def, field_args, widget_attrs) def _is_approver_name_field(self, field_def): """Check if this is an approver name field (to auto-fill).""" @@ -1629,7 +1619,7 @@ def _get_approver_name(self): return self.user.username return "" - def _create_field(self, field_def, field_args, widget_attrs, is_editable): + def _create_field(self, field_def, field_args, widget_attrs): """Create the appropriate Django form field.""" if field_def.field_type == "text": if widget_attrs: @@ -2002,8 +1992,6 @@ def get_enhancements_config(self): validate fields on input/blur instead of waiting for submit, matching the behavior of the original submission form. """ - import json - stage_id = self.approval_task.workflow_stage_id if stage_id is None: stage_fields = [] diff --git a/django_forms_workflows/handlers/file_handler.py b/django_forms_workflows/handlers/file_handler.py index b9c3dad..7650f7b 100644 --- a/django_forms_workflows/handlers/file_handler.py +++ b/django_forms_workflows/handlers/file_handler.py @@ -138,7 +138,7 @@ def rename(self, target_pattern): self.managed_file.file_path = new_path self.managed_file.save(update_fields=["stored_filename", "file_path"]) - logger.info(f"Renamed file from {old_path} to {new_path}") + logger.info("Renamed managed file id=%s", self.managed_file.id) return {"success": True, "message": f"Renamed to {new_filename}"} return {"success": False, "message": f"File not found: {old_path}"} @@ -174,7 +174,7 @@ def move(self, target_pattern): self.managed_file.stored_filename = os.path.basename(new_path) self.managed_file.save(update_fields=["file_path", "stored_filename"]) - logger.info(f"Moved file from {old_path} to {new_path}") + logger.info("Moved managed file id=%s", self.managed_file.id) return {"success": True, "message": f"Moved to {new_path}"} return {"success": False, "message": f"File not found: {old_path}"} @@ -204,7 +204,7 @@ def copy(self, target_pattern): content = f.read() self.storage.save(new_path, content) - logger.info(f"Copied file from {old_path} to {new_path}") + logger.info("Copied managed file id=%s", self.managed_file.id) return {"success": True, "message": f"Copied to {new_path}"} return {"success": False, "message": f"File not found: {old_path}"} @@ -225,7 +225,7 @@ def delete(self): self.managed_file.status_changed_at = timezone.now() self.managed_file.save(update_fields=["status", "status_changed_at"]) - logger.info(f"Deleted file: {file_path}") + logger.info("Deleted managed file id=%s", self.managed_file.id) return {"success": True, "message": f"Deleted {file_path}"} return {"success": False, "message": f"File not found: {file_path}"} diff --git a/django_forms_workflows/migrations/0064_add_reviewer_groups.py b/django_forms_workflows/migrations/0064_add_reviewer_groups.py index 8e3565c..b23ac32 100644 --- a/django_forms_workflows/migrations/0064_add_reviewer_groups.py +++ b/django_forms_workflows/migrations/0064_add_reviewer_groups.py @@ -1,9 +1,7 @@ -import django.db.models.deletion from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [ ("django_forms_workflows", "0063_formdefinition_api_enabled_apitoken"), ] @@ -24,4 +22,3 @@ class Migration(migrations.Migration): ), ), ] - diff --git a/django_forms_workflows/static/django_forms_workflows/js/form-builder-property-editor.js b/django_forms_workflows/static/django_forms_workflows/js/form-builder-property-editor.js index 5e99e66..11cb0c5 100644 --- a/django_forms_workflows/static/django_forms_workflows/js/form-builder-property-editor.js +++ b/django_forms_workflows/static/django_forms_workflows/js/form-builder-property-editor.js @@ -247,12 +247,6 @@ export const propertyEditorMethods = { const conditionalRulesJson = field.conditional_rules ? JSON.stringify(field.conditional_rules, null, 2) : ''; - // Get list of other fields for dropdown - const otherFields = this.fields.filter(f => f.field_name !== field.field_name); - const fieldOptions = otherFields.map(f => - `` - ).join(''); - return `
@@ -350,12 +344,6 @@ export const propertyEditorMethods = { const dependenciesJson = field.field_dependencies.length > 0 ? JSON.stringify(field.field_dependencies, null, 2) : ''; - // Get list of other fields for dropdown - const otherFields = this.fields.filter(f => f.field_name !== field.field_name); - const fieldOptions = otherFields.map(f => - `` - ).join(''); - return `
diff --git a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js index ba40e5c..5475224 100644 --- a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js +++ b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js @@ -2103,7 +2103,6 @@ class WorkflowBuilder { } buildEndProperties(node) { - const data = node.data || {}; return `
This is the terminal node where the workflow ends. @@ -2874,7 +2873,6 @@ class WorkflowBuilder { return 'Workflow starts here'; case 'form': const fieldCount = node.data.field_count || 0; - const formName = node.data.form_name || 'Form'; const isInitial = node.data.is_initial !== false; const isMultiStep = node.data.enable_multi_step && node.data.step_count > 0; @@ -3165,3 +3163,5 @@ class WorkflowBuilder { } } +// This script is loaded before the inline admin bootstrap code. +window.WorkflowBuilder = WorkflowBuilder; diff --git a/django_forms_workflows/tasks.py b/django_forms_workflows/tasks.py index 8289269..c0ab40c 100644 --- a/django_forms_workflows/tasks.py +++ b/django_forms_workflows/tasks.py @@ -1433,6 +1433,7 @@ def send_notification_rules( try: task = ApprovalTask.objects.select_related("workflow_stage").get(id=task_id) except ApprovalTask.DoesNotExist: + # A concurrently deleted task simply leaves notification context unscoped. pass # Build a denormalized "public_comments" list for the template context @@ -1662,6 +1663,7 @@ def send_notification_rules( try: base_context["task"] = ApprovalTask.objects.get(id=task_id) except ApprovalTask.DoesNotExist: + # The notification can still be rendered without optional task context. pass # Pre-fetch User objects for all recipients so templates that diff --git a/django_forms_workflows/utils.py b/django_forms_workflows/utils.py index f812e0a..c6b9814 100644 --- a/django_forms_workflows/utils.py +++ b/django_forms_workflows/utils.py @@ -298,11 +298,12 @@ def sync_ldap_groups(): try: from django_auth_ldap.backend import LDAPBackend - _backend = LDAPBackend() # noqa: F841 - Placeholder for future implementation - # This would need to be implemented based on your LDAP structure # and how you want to sync groups - logger.info("LDAP group sync completed") + logger.info( + "LDAP group sync is available via backend %s but is not configured", + LDAPBackend.__name__, + ) except ImportError: logger.warning("django-auth-ldap not installed, skipping group sync") diff --git a/django_forms_workflows/views.py b/django_forms_workflows/views.py index 5ee3ae5..dc666cc 100644 --- a/django_forms_workflows/views.py +++ b/django_forms_workflows/views.py @@ -412,6 +412,7 @@ def form_submit(request, slug): if isinstance(prev, dict): raw_data.update(prev) except (json.JSONDecodeError, TypeError): + # Ignore malformed client metadata; draft data remains usable. pass draft_obj, created = FormSubmission.objects.update_or_create( @@ -459,6 +460,7 @@ def form_submit(request, slug): for k, v in prev.items(): stashed_files.setdefault(k, v) except (json.JSONDecodeError, TypeError): + # Ignore malformed client metadata and keep newly stashed uploads. pass form = DynamicForm( @@ -789,7 +791,6 @@ def _pipe_answer_tokens(text, form_data): Unresolved tokens are replaced with an empty string so the output is always safe to display or use as a URL. """ - import re def _repl(m): val = form_data.get(m.group(1), "") @@ -968,8 +969,6 @@ def my_submissions(request): active_category = next( (c for c in category_counts if c["slug"] == category_slug), None ) - else: - submissions = base_submissions # --- Form counts within the active category (for the form-level filter bar) --- form_slug = request.GET.get("form", "").strip() @@ -996,7 +995,6 @@ def my_submissions(request): if r["form_definition__slug"] ] if form_slug: - submissions = submissions.filter(form_definition__slug=form_slug) active_form = next((f for f in form_counts if f["slug"] == form_slug), None) # Check if any submissions support bulk export (fast EXISTS) @@ -1442,8 +1440,6 @@ def approval_inbox(request): active_category = next( (c for c in category_counts if c["slug"] == category_slug), None ) - else: - display_tasks = base_tasks # --- Form counts within the active category (for the form-level filter bar) --- form_slug = request.GET.get("form", "").strip() @@ -1470,9 +1466,6 @@ def approval_inbox(request): if r["submission__form_definition__slug"] ] if form_slug: - display_tasks = display_tasks.filter( - submission__form_definition__slug=form_slug - ) active_form = next((f for f in form_counts if f["slug"] == form_slug), None) # --- Form fields for the column picker (only when a specific form is active) --- @@ -2302,8 +2295,6 @@ def completed_approvals(request): # --- Apply optional category filter --- category_slug = request.GET.get("category", "").strip() active_category = None - filtered_submissions = base_submissions - if category_slug: filtered_submissions = base_submissions.filter( form_definition__category__slug=category_slug diff --git a/django_forms_workflows/workflow_builder_views.py b/django_forms_workflows/workflow_builder_views.py index 9ae5740..d763ac6 100644 --- a/django_forms_workflows/workflow_builder_views.py +++ b/django_forms_workflows/workflow_builder_views.py @@ -1074,7 +1074,6 @@ def convert_workflow_to_visual(workflow, form_definition): current_x += horizontal_spacing # ── Sub-workflow node ────────────────────────────────────────────── - sub_wf_config = getattr(workflow, "sub_workflow_config", None) try: sub_wf_config = workflow.sub_workflow_config except SubWorkflowDefinition.DoesNotExist: diff --git a/django_forms_workflows/workflow_engine.py b/django_forms_workflows/workflow_engine.py index 0dc28db..b2f6004 100644 --- a/django_forms_workflows/workflow_engine.py +++ b/django_forms_workflows/workflow_engine.py @@ -384,8 +384,6 @@ def _lookup_by_full_name(user_model, value, stage, submission): first_name__iexact=first_name, last_name__iexact=last_name ) else: - first_name = None - last_name = value qs = user_model.objects.filter(last_name__iexact=value) matches = list(qs[:3]) # fetch up to 3 to detect duplicates cheaply diff --git a/example_project/example/settings.py b/example_project/example/settings.py index d136de0..34d2638 100644 --- a/example_project/example/settings.py +++ b/example_project/example/settings.py @@ -5,7 +5,6 @@ """ from pathlib import Path -import os # Build paths inside the project like this: BASE_DIR / 'subdir'. BASE_DIR = Path(__file__).resolve().parent.parent diff --git a/manage_dev.py b/manage_dev.py index bb44087..ad83037 100644 --- a/manage_dev.py +++ b/manage_dev.py @@ -1,6 +1,5 @@ """Minimal manage.py for running makemigrations inside the package repo.""" -import os import sys # Minimal inline settings so we can run makemigrations without a full project diff --git a/tests/test_file_handler.py b/tests/test_file_handler.py new file mode 100644 index 0000000..d81b416 --- /dev/null +++ b/tests/test_file_handler.py @@ -0,0 +1,52 @@ +import logging +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + +from django_forms_workflows.handlers.file_handler import FileOperationHandler + + +@pytest.mark.parametrize( + ("operation", "target", "expected_log"), + [ + ("rename", "renamed.txt", "Renamed managed file id=42"), + ("move", "private/renamed.txt", "Moved managed file id=42"), + ("copy", "private/copied.txt", "Copied managed file id=42"), + ("delete", None, "Deleted managed file id=42"), + ], +) +def test_file_operations_do_not_log_private_paths( + caplog, operation, target, expected_log +): + source_path = "private/customer-ssn/source.txt" + managed_file = SimpleNamespace( + id=42, + file_path=source_path, + stored_filename="source.txt", + submission=MagicMock(), + save=MagicMock(), + ) + handler = FileOperationHandler(managed_file) + handler.storage = MagicMock() + handler.storage.exists.return_value = True + handler.storage.open.return_value.__enter__.return_value.read.return_value = b"data" + + caplog.set_level( + logging.INFO, logger="django_forms_workflows.handlers.file_handler" + ) + + if target is None: + result = getattr(handler, operation)() + private_paths = (source_path,) + else: + handler.resolver.resolve = MagicMock(return_value=target) + result = getattr(handler, operation)(target) + destination_path = ( + f"private/customer-ssn/{target}" if operation == "rename" else target + ) + private_paths = (source_path, destination_path) + + assert result["success"] is True + assert expected_log in caplog.text + assert all(private_path not in caplog.text for private_path in private_paths) diff --git a/tests/test_views.py b/tests/test_views.py index e439674..2295424 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -2044,7 +2044,7 @@ def confirm_payment(self, *a, **kw): def handle_webhook(self, *a, **kw): pass - def get_client_config(self): + def get_client_config(self, payment_result): return {} def get_receipt_data(self, *a, **kw): diff --git a/tests/test_workflow_engine.py b/tests/test_workflow_engine.py index f3d41a9..e430afa 100644 --- a/tests/test_workflow_engine.py +++ b/tests/test_workflow_engine.py @@ -2,6 +2,8 @@ Tests for django_forms_workflows.workflow_engine. """ +from collections.abc import Callable +from typing import Any from unittest.mock import patch from django.contrib.auth.models import Group, User @@ -40,6 +42,11 @@ def _make_submission(form_def, user, **overrides): return FormSubmission.objects.create(**defaults) +def _execute_on_commit(callback: Callable[[], Any]) -> Any: + """Run a transaction callback immediately in dispatch unit tests.""" + return callback() + + # ── No-workflow (auto-approve) ──────────────────────────────────────────── @@ -73,7 +80,7 @@ def test_workflow_no_approval_required( class TestNotificationDispatch: @patch( "django_forms_workflows.workflow_engine.transaction.on_commit", - side_effect=lambda fn: fn(), + side_effect=_execute_on_commit, ) @patch("django_forms_workflows.tasks.send_notification_rules.delay") def test_notification_rules_dispatch_on_commit( @@ -88,7 +95,7 @@ def test_notification_rules_dispatch_on_commit( class TestWebhookDispatch: @patch( "django_forms_workflows.workflow_engine.transaction.on_commit", - side_effect=lambda fn: fn(), + side_effect=_execute_on_commit, ) @patch("django_forms_workflows.tasks.dispatch_workflow_webhooks.delay") def test_webhooks_dispatch_on_commit(self, mock_delay, mock_on_commit, submission): @@ -105,7 +112,7 @@ def test_webhooks_dispatch_on_commit(self, mock_delay, mock_on_commit, submissio @patch( "django_forms_workflows.workflow_engine.transaction.on_commit", - side_effect=lambda fn: fn(), + side_effect=_execute_on_commit, ) @patch("django_forms_workflows.tasks.send_notification_rules.delay") @patch("django_forms_workflows.tasks.dispatch_workflow_webhooks.delay") @@ -148,7 +155,7 @@ def test_task_request_dispatches_task_created( @patch( "django_forms_workflows.workflow_engine.transaction.on_commit", - side_effect=lambda fn: fn(), + side_effect=_execute_on_commit, ) @patch("django_forms_workflows.tasks.send_notification_rules") @patch("django_forms_workflows.tasks.dispatch_workflow_webhooks")