diff --git a/app/modules/documents/ui.py b/app/modules/documents/ui.py index 68b4777..9069693 100644 --- a/app/modules/documents/ui.py +++ b/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=f"/documents/engagements/{engagement_id}?uploaded=1", 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=return_target if return_to else f"/documents/engagements/{engagement_id}?uploaded=1", status_code=303) finally: db.close() diff --git a/app/modules/services/engagement_resources.py b/app/modules/services/engagement_resources.py index da50a1e..092e620 100644 --- a/app/modules/services/engagement_resources.py +++ b/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 - folder convention already used by the ERP Local Agent. Task-category uploads map - to an existing task so evidence/AQMM/document controls remain authoritative. + No new persistence is introduced. Accounting paths continue to follow the same + FY/client convention used by the ERP Local Agent. Category uploads use existing + 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] = {} - for task in tasks: - 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()] + categories = _task_category_options(db, engagement, tasks) + default_upload_task_id = next((row["task_id"] for row in categories if row.get("task_id")), None) 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, } diff --git a/app/modules/services/templates/services/engagements/_engagement_resources.html b/app/modules/services/templates/services/engagements/_engagement_resources.html index b6121ea..a731a5d 100644 --- a/app/modules/services/templates/services/engagements/_engagement_resources.html +++ b/app/modules/services/templates/services/engagements/_engagement_resources.html @@ -60,16 +60,23 @@
Upload from Engagement Page
-

Choose the task category so the file remains linked to the existing task/evidence workflow.

- {% if resources.default_upload_task_id %} -
+

Select Tally Data or a service task category. Category uploads stay linked to the existing task/evidence workflow.

+
- - + + {% for category in resources.task_categories %} + + {% endfor %} + {% if resources.task_categories %} +

Categories come from this service's Task Category master; older engagements are mapped through their task templates where needed.

+ {% endif %}
@@ -107,11 +114,9 @@
+
Tally Data is stored at engagement level and remains separate from task-category evidence.
- {% else %} -
No task is available to receive an engagement upload.
- {% endif %}
@@ -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();