Fix engagement task categories and add Tally Data upload

app/modules/documents/ui.py

@@ -359,14 +359,14 @@ def upload_engagement_document(
     remarks: str | None = Form(None),
     existing_document_id: str | None = Form(None),
     udin_required: str | None = Form(None),
-    evidence_type: str | None = Form(None),
-    evidence_description: str | None = Form(None),
     file: UploadFile = File(...),
+    return_to: str = Form(""),
     csrf_token: str = Form(...),
 ):
     validate_csrf(request, csrf_token)
     db = CommonSessionLocal()
     try:
+        return_target = _safe_documents_return_url(return_to, f"/documents/engagements/{engagement_id}")
         user, response = _require_user(request, db, "documents.upload")
         if response:
             return response
@@ -378,9 +378,9 @@ def upload_engagement_document(
         if not engagement or not user_can_upload_to_engagement(db, user, engagement, scope):
             return _redirect_denied()
         if is_row_financial_year_locked(db, engagement):
-            return RedirectResponse(url=f"/documents/engagements/{engagement.id}?year_locked=1", status_code=303)
+            return RedirectResponse(url=return_target if return_to else f"/documents/engagements/{engagement.id}?year_locked=1", status_code=303)
         if not file or not file.filename:
-            return RedirectResponse(url=f"/documents/engagements/{engagement_id}?error=missing_file", status_code=303)
+            return RedirectResponse(url=return_target if return_to else f"/documents/engagements/{engagement_id}?error=missing_file", status_code=303)
         try:
             doc = save_uploaded_revision(
                 db,
@@ -393,10 +393,7 @@ def upload_engagement_document(
                 user=user,
                 existing_document_id=int(existing_document_id) if existing_document_id else None,
                 udin_required=_bool_from_form(udin_required),
-                evidence_type=evidence_type,
-                evidence_description=evidence_description,
             )
-            recalculate_task_aqmm_status(db, task)
             log_document_access(db, action="upload", result="success", user=user, request=request, document=doc)
             db.commit()
         except Exception as exc:
@@ -408,8 +405,8 @@ def upload_engagement_document(
             except Exception:
                 db.rollback()
                 logger.exception("Unable to write document upload failure audit log for engagement_id=%s", engagement_id)
-            return RedirectResponse(url=f"/documents/engagements/{engagement_id}?error=upload_failed", status_code=303)
+            return RedirectResponse(url=return_target if return_to else f"/documents/engagements/{engagement_id}?error=upload_failed", status_code=303)
-        return RedirectResponse(url=f"/documents/engagements/{engagement_id}?uploaded=1", status_code=303)
+        return RedirectResponse(url=return_target if return_to else f"/documents/engagements/{engagement_id}?uploaded=1", status_code=303)
     finally:
         db.close()
 

app/modules/services/engagement_resources.py

@@ -4,7 +4,10 @@ import re
 from pathlib import Path
 from typing import Any, Iterable
 
+from sqlalchemy import select
+
 from app.modules.documents.services import client_folder_parts, get_active_storage_node_for_branch, sanitize_segment
+from app.modules.services.models import FirmServiceTaskTemplate, ServiceTaskCategory
 
 
 def _join_local_path(root: str, relative: str) -> str:
@@ -16,12 +19,129 @@ def _join_local_path(root: str, relative: str) -> str:
     return root + "/" + relative.lstrip("/")
 
 
+def _task_category_options(db, engagement, tasks: list[Any]) -> list[dict[str, Any]]:
+    """Return the service's real task-category master mapped to this engagement's tasks.
+
+    Older task instances may not contain the task_category snapshot even though their
+    FirmServiceTaskTemplate has since been linked to ServiceTaskCategory.  Therefore
+    category resolution deliberately uses both the generated task snapshot and the
+    current template/category master, without changing any historical task rows.
+    """
+    catalogue_id = int(getattr(engagement, "service_catalogue_id", 0) or 0)
+    tenant_id = int(getattr(engagement, "tenant_id", 0) or 0)
+    if not catalogue_id:
+        return []
+
+    template_ids = {
+        int(getattr(task, "firm_task_template_id", 0) or 0)
+        for task in tasks
+        if int(getattr(task, "firm_task_template_id", 0) or 0)
+    }
+    templates_by_id: dict[int, Any] = {}
+    if template_ids:
+        rows = db.execute(
+            select(FirmServiceTaskTemplate).where(FirmServiceTaskTemplate.id.in_(template_ids))
+        ).scalars().all()
+        templates_by_id = {int(row.id): row for row in rows}
+
+    task_by_category_id: dict[int, int] = {}
+    task_by_category_name: dict[str, int] = {}
+    legacy_names: dict[str, tuple[str, int]] = {}
+
+    for task in sorted(tasks, key=lambda x: (int(getattr(x, "sequence_no", 0) or 0), int(getattr(x, "id", 0) or 0))):
+        task_id = int(getattr(task, "id", 0) or 0)
+        if not task_id:
+            continue
+        template_id = int(getattr(task, "firm_task_template_id", 0) or 0)
+        template = templates_by_id.get(template_id)
+        category_id = int(getattr(template, "task_category_id", 0) or 0) if template else 0
+        snapshot_name = str(getattr(task, "task_category", "") or "").strip()
+        template_name = str(getattr(template, "task_category", "") or "").strip() if template else ""
+        category_name = snapshot_name or template_name
+
+        if category_id and category_id not in task_by_category_id:
+            task_by_category_id[category_id] = task_id
+        if category_name:
+            key = category_name.casefold()
+            task_by_category_name.setdefault(key, task_id)
+            legacy_names.setdefault(key, (category_name, task_id))
+
+    # Firm categories are authoritative for a firm engagement.  If none exist,
+    # retain support for the system/default category master.
+    firm_rows = db.execute(
+        select(ServiceTaskCategory).where(
+            ServiceTaskCategory.service_catalogue_id == catalogue_id,
+            ServiceTaskCategory.tenant_id == tenant_id,
+            ServiceTaskCategory.is_active.is_(True),
+        ).order_by(ServiceTaskCategory.sort_order.asc(), ServiceTaskCategory.name.asc(), ServiceTaskCategory.id.asc())
+    ).scalars().all()
+    master_rows = list(firm_rows)
+    if not master_rows:
+        master_rows = db.execute(
+            select(ServiceTaskCategory).where(
+                ServiceTaskCategory.service_catalogue_id == catalogue_id,
+                ServiceTaskCategory.tenant_id.is_(None),
+                ServiceTaskCategory.is_active.is_(True),
+            ).order_by(ServiceTaskCategory.sort_order.asc(), ServiceTaskCategory.name.asc(), ServiceTaskCategory.id.asc())
+        ).scalars().all()
+
+    options: list[dict[str, Any]] = []
+    seen_names: set[str] = set()
+    for category in master_rows:
+        name = str(getattr(category, "name", "") or "").strip()
+        if not name:
+            continue
+        key = name.casefold()
+        task_id = task_by_category_id.get(int(category.id)) or task_by_category_name.get(key)
+        options.append({
+            "id": int(category.id),
+            "code": str(getattr(category, "code", "") or ""),
+            "name": name,
+            "task_id": task_id,
+            "available": bool(task_id),
+            "source": "master",
+        })
+        seen_names.add(key)
+
+    # Do not hide historical categories if a legacy task still has one that is no
+    # longer present in the active master.
+    for key, (name, task_id) in legacy_names.items():
+        if key in seen_names:
+            continue
+        options.append({
+            "id": None,
+            "code": "",
+            "name": name,
+            "task_id": task_id,
+            "available": True,
+            "source": "legacy",
+        })
+        seen_names.add(key)
+
+    # Preserve the old behaviour only as a final fallback for genuinely
+    # uncategorised engagements.
+    if not options and tasks:
+        first_task_id = int(getattr(tasks[0], "id", 0) or 0)
+        if first_task_id:
+            options.append({
+                "id": None,
+                "code": "",
+                "name": "General Workflow",
+                "task_id": first_task_id,
+                "available": True,
+                "source": "fallback",
+            })
+    return options
+
+
 def build_engagement_resource_context(db, engagement, tasks: Iterable[Any], documents: Iterable[Any]) -> dict[str, Any]:
     """Presentation-only engagement resource data using existing storage/task models.
 
-    No new persistence is introduced here. Accounting paths follow the same FY/client
+    No new persistence is introduced. Accounting paths continue to follow the same
-    folder convention already used by the ERP Local Agent. Task-category uploads map
+    FY/client convention used by the ERP Local Agent. Category uploads use existing
-    to an existing task so evidence/AQMM/document controls remain authoritative.
+    task instances so evidence/AQMM controls remain authoritative. Tally Data is an
+    engagement-level document destination and therefore is intentionally not forced
+    into an unrelated task category.
     """
     tasks = list(tasks or [])
     documents = list(documents or [])
@@ -50,13 +170,8 @@ def build_engagement_resource_context(db, engagement, tasks: Iterable[Any], docu
             storage_node_name = str(getattr(node, "node_name", "") or "").strip()
             accounting_local_path = _join_local_path(storage_root_path, accounting_relative_path)
 
-    category_map: dict[str, int] = {}
+    categories = _task_category_options(db, engagement, tasks)
-    for task in tasks:
+    default_upload_task_id = next((row["task_id"] for row in categories if row.get("task_id")), None)
-        category = str(getattr(task, "task_category", "") or "").strip() or "General Workflow"
-        task_id = int(getattr(task, "id", 0) or 0)
-        if task_id and category not in category_map:
-            category_map[category] = task_id
-    categories = [{"name": name, "task_id": task_id} for name, task_id in category_map.items()]
 
     document_options = []
     for doc in documents:
@@ -80,6 +195,6 @@ def build_engagement_resource_context(db, engagement, tasks: Iterable[Any], docu
         "storage_root_path": storage_root_path,
         "storage_node_name": storage_node_name,
         "task_categories": categories,
-        "default_upload_task_id": categories[0]["task_id"] if categories else None,
+        "default_upload_task_id": default_upload_task_id,
         "document_options": document_options,
     }

app/modules/services/templates/services/engagements/_engagement_resources.html

@@ -60,16 +60,23 @@
 
     <div class="rounded-2xl border border-slate-200 bg-slate-50 p-4">
       <div class="text-sm font-semibold text-slate-900">Upload from Engagement Page</div>
-      <p class="mt-1 text-xs text-slate-500">Choose the task category so the file remains linked to the existing task/evidence workflow.</p>
+      <p class="mt-1 text-xs text-slate-500">Select Tally Data or a service task category. Category uploads stay linked to the existing task/evidence workflow.</p>
-      {% if resources.default_upload_task_id %}
+      <form method="post" enctype="multipart/form-data" action="/documents/engagements/{{ engagement_id }}/upload" class="mt-4 space-y-3" data-engagement-upload-form data-engagement-id="{{ engagement_id }}">
-      <form method="post" enctype="multipart/form-data" action="/documents/tasks/{{ resources.default_upload_task_id }}/upload" class="mt-4 space-y-3" data-engagement-upload-form>
         <input type="hidden" name="csrf_token" value="{{ csrf_token }}">
         <input type="hidden" name="return_to" value="{{ return_to }}">
         <div>
-          <label class="mb-1 block text-xs font-semibold uppercase tracking-wide text-slate-500">Task Category</label>
+          <label class="mb-1 block text-xs font-semibold uppercase tracking-wide text-slate-500">Upload To</label>
-          <select class="w-full rounded-xl border border-slate-300 bg-white px-3 py-2 text-sm" data-upload-task-select>
+          <select class="w-full rounded-xl border border-slate-300 bg-white px-3 py-2 text-sm" data-upload-target-select>
-            {% for category in resources.task_categories %}<option value="{{ category.task_id }}">{{ category.name }}</option>{% endfor %}
+            <option value="tally" data-kind="tally">Tally Data</option>
+            {% for category in resources.task_categories %}
+            <option value="category-{{ loop.index }}" data-kind="category" data-task-id="{{ category.task_id or '' }}" {% if not category.available %}disabled{% endif %}>
+              {{ category.name }}{% if not category.available %} — no task in this engagement{% endif %}
+            </option>
+            {% endfor %}
           </select>
+          {% if resources.task_categories %}
+          <p class="mt-1 text-[11px] text-slate-500">Categories come from this service's Task Category master; older engagements are mapped through their task templates where needed.</p>
+          {% endif %}
         </div>
         <div>
           <label class="mb-1 block text-xs font-semibold uppercase tracking-wide text-slate-500">Upload Action</label>
@@ -107,11 +114,9 @@
           <label class="mb-1 block text-xs font-semibold uppercase tracking-wide text-slate-500">Description / Remarks</label>
           <input name="remarks" class="w-full rounded-xl border border-slate-300 bg-white px-3 py-2 text-sm" placeholder="Optional">
         </div>
+        <div data-upload-target-note class="rounded-xl border border-indigo-100 bg-indigo-50 px-3 py-2 text-xs text-indigo-700">Tally Data is stored at engagement level and remains separate from task-category evidence.</div>
         <button class="w-full rounded-xl bg-brand-600 px-4 py-2.5 text-sm font-semibold text-white hover:bg-brand-700">Upload Document</button>
       </form>
-      {% else %}
-      <div class="mt-4 rounded-xl border border-dashed border-slate-300 bg-white p-5 text-center text-sm text-slate-500">No task is available to receive an engagement upload.</div>
-      {% endif %}
     </div>
   </div>
 </section>
@@ -120,12 +125,24 @@
 (function () {
   const form = document.querySelector('[data-engagement-upload-form]');
   if (form) {
-    const taskSelect = form.querySelector('[data-upload-task-select]');
+    const targetSelect = form.querySelector('[data-upload-target-select]');
     const mode = form.querySelector('[data-upload-mode]');
     const wrap = form.querySelector('[data-existing-document-wrap]');
     const existing = form.querySelector('[data-existing-document-select]');
+    const note = form.querySelector('[data-upload-target-note]');
+    const engagementId = form.getAttribute('data-engagement-id');
+
     const updateAction = function () {
-      form.action = '/documents/tasks/' + taskSelect.value + '/upload';
+      const option = targetSelect.options[targetSelect.selectedIndex];
+      const kind = option ? option.getAttribute('data-kind') : 'tally';
+      const taskId = option ? option.getAttribute('data-task-id') : '';
+      if (kind === 'category' && taskId) {
+        form.action = '/documents/tasks/' + taskId + '/upload';
+        note.textContent = 'This file will be linked to the selected category through its existing engagement task, preserving evidence and AQMM behaviour.';
+      } else {
+        form.action = '/documents/engagements/' + engagementId + '/upload';
+        note.textContent = 'Tally Data is stored at engagement level and remains separate from task-category evidence.';
+      }
     };
     const updateMode = function () {
       const useExisting = mode.value !== 'new';
@@ -133,7 +150,7 @@
       existing.required = useExisting;
       if (!useExisting) existing.value = '';
     };
-    taskSelect.addEventListener('change', updateAction);
+    targetSelect.addEventListener('change', updateAction);
     mode.addEventListener('change', updateMode);
     updateAction();
     updateMode();