Fix GitHub security and code quality findings

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 = []

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
 

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 = []

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}"}
 

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):
             ),
         ),
     ]
-

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 =>
-            `<option value="${f.field_name}">${this.escapeHtml(f.field_label)} (${f.field_name})</option>`
-        ).join('');
-
         return `
             <div class="row g-3">
                 <div class="col-12">
@@ -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 =>
-            `<option value="${f.field_name}">${this.escapeHtml(f.field_label)} (${f.field_name})</option>`
-        ).join('');
-
         return `
             <div class="row g-3">
                 <div class="col-12">

django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js

@@ -2103,7 +2103,6 @@ class WorkflowBuilder {
     }
 
     buildEndProperties(node) {
-        const data = node.data || {};
         return `
             <div class="alert alert-info">
                 <i class="bi bi-info-circle"></i> 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;

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

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")

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

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:

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

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

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