From 9068b523bc93bca29e90527fecb0df90ce7a33b3 Mon Sep 17 00:00:00 2001 From: A R R R Associates Date: Thu, 3 Sep 2026 14:23:14 +0530 Subject: [PATCH] Add engagement financial year correction workflow --- .../services/engagement_fy_correction.py | 299 ++++++++++++++++++ app/modules/services/engagements_ui.py | 168 ++++++++++ .../services/engagements/detail.html | 31 ++ .../templates/services/engagements/list.html | 42 ++- 4 files changed, 533 insertions(+), 7 deletions(-) create mode 100644 app/modules/services/engagement_fy_correction.py diff --git a/app/modules/services/engagement_fy_correction.py b/app/modules/services/engagement_fy_correction.py new file mode 100644 index 0000000..738e160 --- /dev/null +++ b/app/modules/services/engagement_fy_correction.py @@ -0,0 +1,299 @@ +from __future__ import annotations + +import json +import re +from dataclasses import dataclass + +from fastapi import Request +from sqlalchemy import select +from sqlalchemy.orm import Session + +from app.modules.core.audit.models import AuditLog +from app.modules.core.tenancy.year_control import is_financial_year_locked, is_row_financial_year_locked +from app.modules.documents.models import EngagementDocument +from app.modules.services.client_services import assessment_year_from_financial_year, period_choices_for_service +from app.modules.services.due_dates import apply_due_date_rule_to_subscription +from app.modules.services.models import ( + ClientServiceSubscription, + ClientServiceTaskInstance, + EngagementClosureChecklist, +) + +_FINANCIAL_YEAR_RE = re.compile(r"^(\d{4})-(\d{2})$") +_CLOSED_ENGAGEMENT_STATUSES = {"completed", "cancelled", "inactive"} + + +@dataclass(frozen=True) +class EngagementFYCorrectionResult: + subscription_id: int + old_financial_year: str + new_financial_year: str + old_period_label: str + new_period_label: str + task_count: int + document_count: int + + +def validate_financial_year(value: str | None) -> str: + raw = (value or "").strip() + match = _FINANCIAL_YEAR_RE.fullmatch(raw) + if not match: + raise ValueError("Financial year must be in YYYY-YY format, for example 2025-26.") + start_year = int(match.group(1)) + expected_suffix = str(start_year + 1)[-2:] + if match.group(2) != expected_suffix: + raise ValueError("Financial year end year does not match the start year.") + return f"{start_year:04d}-{expected_suffix}" + + +def _remap_period_label(row: ClientServiceSubscription, target_financial_year: str) -> str: + recurrence = (getattr(getattr(row, "catalogue", None), "recurrence_type", None) or "one_time").strip().lower() + current = (getattr(row, "period_label", None) or "").strip() + choices = period_choices_for_service(target_financial_year, recurrence) + valid_codes = [code for code, _label in choices] + + if recurrence == "monthly": + # Period codes are YYYY-MM. Keep the same month while moving it to the + # corresponding month inside the corrected financial year. + try: + month = int(current.rsplit("-", 1)[1]) + except (ValueError, IndexError): + raise ValueError("The existing monthly period is invalid and cannot be remapped automatically.") + target_start = int(target_financial_year.split("-", 1)[0]) + target_year = target_start if month >= 4 else target_start + 1 + candidate = f"{target_year:04d}-{month:02d}" + if candidate not in valid_codes: + raise ValueError("The monthly period cannot be mapped to the corrected financial year.") + return candidate + + if recurrence == "quarterly": + if current not in valid_codes: + raise ValueError("The existing quarter is invalid for the corrected financial year.") + return current + + return "" + + +def _engagement_has_finalised_documents(db: Session, subscription_id: int) -> bool: + rows = db.execute( + select(EngagementDocument).where( + EngagementDocument.engagement_id == subscription_id, + EngagementDocument.is_deleted.is_(False), + ) + ).scalars().all() + for document in rows: + if (getattr(document, "final_release_status", None) or "draft").strip().lower() == "released": + return True + if (getattr(document, "udin_number", None) or "").strip(): + return True + return False + + +def validate_engagement_can_change_financial_year( + db: Session, + *, + row: ClientServiceSubscription, + target_financial_year: str, +) -> None: + if getattr(row, "is_locked", False): + raise ValueError("Locked engagements cannot be moved to another financial year.") + if is_row_financial_year_locked(db, row): + raise ValueError(f"Source FY {row.financial_year} is locked and cannot be changed.") + if is_financial_year_locked(db, tenant_id=row.tenant_id, year_code=target_financial_year): + raise ValueError(f"Target FY {target_financial_year} is locked and cannot receive an engagement.") + if not getattr(row, "is_active", True): + raise ValueError("Inactive engagements cannot be moved to another financial year.") + status = (getattr(row, "status", None) or "active").strip().lower() + if status in _CLOSED_ENGAGEMENT_STATUSES: + raise ValueError("Only open engagements can have their financial year corrected.") + + closure = db.execute( + select(EngagementClosureChecklist).where( + EngagementClosureChecklist.subscription_id == row.id + ) + ).scalar_one_or_none() + if closure and (closure.closure_status or "").strip().lower() == "approved": + raise ValueError("The engagement closure is already approved. Reopen it before correcting the financial year.") + if _engagement_has_finalised_documents(db, row.id): + raise ValueError("A released final document or UDIN exists. Reopen/correct the finalisation workflow before changing FY.") + + +def _duplicate_engagement( + db: Session, + *, + row: ClientServiceSubscription, + target_financial_year: str, + target_period_label: str, +) -> ClientServiceSubscription | None: + return db.execute( + select(ClientServiceSubscription).where( + ClientServiceSubscription.tenant_id == row.tenant_id, + ClientServiceSubscription.service_catalogue_id == row.service_catalogue_id, + ClientServiceSubscription.scope_key == row.scope_key, + ClientServiceSubscription.financial_year == target_financial_year, + ClientServiceSubscription.period_label == target_period_label, + ClientServiceSubscription.id != row.id, + ) + ).scalar_one_or_none() + + +def _add_audit_log( + db: Session, + *, + request: Request | None, + row: ClientServiceSubscription, + actor, + old_financial_year: str, + new_financial_year: str, + old_period_label: str, + new_period_label: str, + reason: str, + task_count: int, + document_count: int, +) -> None: + ip_address = request.client.host if request and request.client else None + user_agent = request.headers.get("user-agent") if request else None + client_name = getattr(getattr(row, "client", None), "client_name", None) or f"Client {row.client_id}" + service_name = getattr(getattr(row, "catalogue", None), "service_name", None) or f"Service {row.service_catalogue_id}" + db.add( + AuditLog( + actor_user_id=getattr(actor, "id", None), + actor_email=getattr(actor, "email", None), + actor_tenant_id=getattr(actor, "tenant_id", None), + actor_branch_id=getattr(actor, "branch_id", None), + action="engagement.financial_year_corrected", + entity_type="ClientServiceSubscription", + entity_id=str(row.id), + entity_name=f"{client_name} - {service_name}", + status="success", + target_tenant_id=row.tenant_id, + target_branch_id=row.branch_id, + ip_address=ip_address, + user_agent=user_agent, + details_json=json.dumps( + { + "old_financial_year": old_financial_year, + "new_financial_year": new_financial_year, + "old_assessment_year": assessment_year_from_financial_year(old_financial_year), + "new_assessment_year": assessment_year_from_financial_year(new_financial_year), + "old_period_label": old_period_label, + "new_period_label": new_period_label, + "reason": reason, + "task_instances_updated": task_count, + "engagement_documents_updated": document_count, + "physical_document_paths_moved": False, + }, + ensure_ascii=False, + sort_keys=True, + ), + ) + ) + + +def correct_engagement_financial_year( + db: Session, + *, + row: ClientServiceSubscription, + target_financial_year: str, + actor, + reason: str, + request: Request | None = None, +) -> EngagementFYCorrectionResult: + target_fy = validate_financial_year(target_financial_year) + reason_text = (reason or "").strip() + if not reason_text: + raise ValueError("A correction reason is required for the audit trail.") + + old_fy = validate_financial_year(row.financial_year) + if target_fy == old_fy: + raise ValueError("The corrected financial year is the same as the existing financial year.") + + validate_engagement_can_change_financial_year(db, row=row, target_financial_year=target_fy) + old_period = (row.period_label or "").strip() + new_period = _remap_period_label(row, target_fy) + + duplicate = _duplicate_engagement( + db, + row=row, + target_financial_year=target_fy, + target_period_label=new_period, + ) + if duplicate: + raise ValueError( + f"A matching engagement already exists in FY {target_fy}" + + (f" for period {new_period}" if new_period else "") + + "." + ) + + # Pre-flight the task unique key before mutating anything. A partially + # inconsistent legacy subscription must be repaired manually rather than + # losing execution/review history. + target_task = db.execute( + select(ClientServiceTaskInstance.id).where( + ClientServiceTaskInstance.subscription_id == row.id, + ClientServiceTaskInstance.financial_year == target_fy, + ).limit(1) + ).scalar_one_or_none() + if target_task is not None: + raise ValueError( + f"This engagement already has a task instance tagged to FY {target_fy}. " + "The mixed-year task data must be reviewed before FY correction." + ) + + new_ay = assessment_year_from_financial_year(target_fy) + tasks = db.execute( + select(ClientServiceTaskInstance).where( + ClientServiceTaskInstance.subscription_id == row.id + ) + ).scalars().all() + documents = db.execute( + select(EngagementDocument).where( + EngagementDocument.engagement_id == row.id, + EngagementDocument.is_deleted.is_(False), + ) + ).scalars().all() + + row.financial_year = target_fy + row.assessment_year = new_ay + row.period_label = new_period + row.updated_by_user_id = getattr(actor, "id", None) + + for task in tasks: + task.financial_year = target_fy + task.assessment_year = new_ay + task.period_label = new_period + task.updated_by_user_id = getattr(actor, "id", None) + + # Document metadata follows the corrected engagement. Existing physical + # revision paths are deliberately preserved so no already-uploaded evidence + # is orphaned; future uploads use the corrected engagement FY. + for document in documents: + document.financial_year = target_fy + document.assessment_year = new_ay + document.updated_by_user_id = getattr(actor, "id", None) + + apply_due_date_rule_to_subscription(db, row, force=True) + _add_audit_log( + db, + request=request, + row=row, + actor=actor, + old_financial_year=old_fy, + new_financial_year=target_fy, + old_period_label=old_period, + new_period_label=new_period, + reason=reason_text, + task_count=len(tasks), + document_count=len(documents), + ) + db.flush() + + return EngagementFYCorrectionResult( + subscription_id=row.id, + old_financial_year=old_fy, + new_financial_year=target_fy, + old_period_label=old_period, + new_period_label=new_period, + task_count=len(tasks), + document_count=len(documents), + ) diff --git a/app/modules/services/engagements_ui.py b/app/modules/services/engagements_ui.py index 623ffad..961cd34 100644 --- a/app/modules/services/engagements_ui.py +++ b/app/modules/services/engagements_ui.py @@ -54,6 +54,10 @@ from app.modules.services.client_services import ( from app.modules.core.rbac.deps import get_user_permissions, get_user_roles from app.modules.core.rbac.permission_guard import require_permission from app.modules.core.tenancy.year_control import redirect_if_financial_year_locked, is_row_financial_year_locked +from app.modules.services.engagement_fy_correction import ( + correct_engagement_financial_year, + validate_financial_year, +) router = APIRouter(prefix="/services/engagements", tags=["services-engagements-ui"]) @@ -126,6 +130,25 @@ def _can_lock_engagements(db, user) -> bool: return bool(roles.intersection({"Firm Admin", "Partner"})) +def _can_correct_engagement_financial_year(db, user) -> bool: + roles = set(get_user_roles(db, user.id)) + return bool(roles.intersection({"Firm Admin", "Partner"})) + + +def _user_can_correct_engagement_financial_year(db, user, row: ClientServiceSubscription) -> bool: + roles = set(get_user_roles(db, user.id)) + if "Firm Admin" in roles: + return True + if "Partner" in roles: + client = getattr(row, "client", None) + return bool( + getattr(row, "assigned_partner_user_id", None) == user.id + or getattr(row, "performing_partner_user_id", None) == user.id + or getattr(client, "partner_id", None) == user.id + ) + return False + + def _user_can_lock_subscription(db, user, row: ClientServiceSubscription) -> bool: roles = set(get_user_roles(db, user.id)) if "Firm Admin" in roles: @@ -217,6 +240,9 @@ def subscription_list( bulk_created: int = 0, bulk_existing: int = 0, bulk_skipped: int = 0, + fy_corrected: int = 0, + fy_skipped: int = 0, + fy_error: str = "", ): db = CommonSessionLocal() try: @@ -263,6 +289,10 @@ def subscription_list( bulk_skipped_count=bulk_skipped, can_manage=_can_manage_client_services(db, user), can_lock_engagements=_can_lock_engagements(db, user), + can_correct_engagement_fy=_can_correct_engagement_financial_year(db, user), + fy_corrected_count=fy_corrected, + fy_skipped_count=fy_skipped, + fy_error=fy_error, ) finally: db.close() @@ -773,6 +803,83 @@ def subscription_bulk_lock( db.close() +@router.post("/bulk-correct-financial-year") +def subscription_bulk_correct_financial_year( + request: Request, + subscription_ids: list[int] = Form([]), + target_financial_year: str = Form(...), + correction_reason: str = Form(...), + financial_year: str = Form(""), + q: str = Form(""), + include_inactive: str | None = Form(None), + csrf_token: str = Form(...), +): + validate_csrf(request, csrf_token) + db = CommonSessionLocal() + corrected = 0 + skipped = 0 + first_error = "" + target_fy = (target_financial_year or "").strip() + try: + user = get_current_user(request, db=db) + if not user: + return RedirectResponse(url="/login", status_code=303) + if not _can_correct_engagement_financial_year(db, user): + return _redirect_denied() + + tenant_id = _active_tenant_id(request, user) + try: + target_fy = validate_financial_year(target_fy) + except ValueError as exc: + from urllib.parse import quote_plus + source_fy = normalize_financial_year(financial_year or _active_financial_year(request)) + return RedirectResponse( + url=f"/services/engagements?financial_year={source_fy}&fy_error={quote_plus(str(exc))}", + status_code=303, + ) + + for subscription_id in [int(value) for value in subscription_ids if value]: + row = get_subscription(db, subscription_id=subscription_id, tenant_id=tenant_id) + if not row or not _user_can_correct_engagement_financial_year(db, user, row): + skipped += 1 + if not first_error: + first_error = "One or more selected engagements were unavailable or not permitted." + continue + try: + with db.begin_nested(): + correct_engagement_financial_year( + db, + row=row, + target_financial_year=target_fy, + actor=user, + reason=correction_reason, + request=request, + ) + corrected += 1 + except ValueError as exc: + skipped += 1 + if not first_error: + first_error = str(exc) + + db.commit() + from urllib.parse import quote_plus + source_fy = normalize_financial_year(financial_year or _active_financial_year(request)) + params = [ + f"financial_year={source_fy}", + f"fy_corrected={corrected}", + f"fy_skipped={skipped}", + ] + if q.strip(): + params.append(f"q={quote_plus(q.strip())}") + if include_inactive: + params.append("include_inactive=true") + if first_error: + params.append(f"fy_error={quote_plus(first_error)}") + return RedirectResponse(url=f"/services/engagements?{'&'.join(params)}", status_code=303) + finally: + db.close() + + @router.get("/{subscription_id}") def subscription_detail(request: Request, subscription_id: int): db = CommonSessionLocal() @@ -828,6 +935,10 @@ def subscription_detail(request: Request, subscription_id: int): closure_checklist=closure_checklist, closure_summary=closure_summary, can_manage=_can_manage_client_services(db, user), + can_correct_engagement_fy=( + _can_correct_engagement_financial_year(db, user) + and _user_can_correct_engagement_financial_year(db, user, row) + ), ) finally: db.close() @@ -1101,6 +1212,63 @@ def subscription_closure_reopen( db.close() +@router.post("/{subscription_id}/correct-financial-year") +def subscription_correct_financial_year( + request: Request, + subscription_id: int, + target_financial_year: str = Form(...), + correction_reason: str = Form(...), + csrf_token: str = Form(...), +): + validate_csrf(request, csrf_token) + db = CommonSessionLocal() + try: + user = get_current_user(request, db=db) + if not user: + return RedirectResponse(url="/login", status_code=303) + if not _can_correct_engagement_financial_year(db, user): + return _redirect_denied() + + tenant_id = _active_tenant_id(request, user) + row = get_subscription(db, subscription_id=subscription_id, tenant_id=tenant_id) + if not row: + return RedirectResponse(url="/services/engagements", status_code=303) + if not _user_can_correct_engagement_financial_year(db, user, row): + return _redirect_denied() + + old_fy = row.financial_year + try: + result = correct_engagement_financial_year( + db, + row=row, + target_financial_year=target_financial_year, + actor=user, + reason=correction_reason, + request=request, + ) + db.commit() + except ValueError as exc: + db.rollback() + from urllib.parse import quote_plus + return RedirectResponse( + url=f"/services/engagements/{subscription_id}?fy_error={quote_plus(str(exc))}", + status_code=303, + ) + + # The active session FY may still be the old year. Return to the corrected + # FY list rather than silently changing the user's global year context. + from urllib.parse import quote_plus + return RedirectResponse( + url=( + f"/services/engagements?financial_year={result.new_financial_year}" + f"&fy_corrected=1&fy_from={quote_plus(old_fy)}" + ), + status_code=303, + ) + finally: + db.close() + + @router.get("/{subscription_id}/edit") def subscription_edit_page(request: Request, subscription_id: int): db = CommonSessionLocal() diff --git a/app/modules/services/templates/services/engagements/detail.html b/app/modules/services/templates/services/engagements/detail.html index 4362d26..bcd2c59 100644 --- a/app/modules/services/templates/services/engagements/detail.html +++ b/app/modules/services/templates/services/engagements/detail.html @@ -8,6 +8,37 @@ {% if can_manage and not row.is_locked %}Edit{% endif %} + {% if request.query_params.get('fy_error') %} +
{{ request.query_params.get('fy_error') }}
+ {% endif %} + + {% if can_correct_engagement_fy and not row.is_locked and row.is_active and row.status not in ['completed', 'cancelled', 'inactive'] %} +
+
+
+

Correct Financial Year

+

Partner/Firm Admin correction for an engagement created under the wrong FY. Existing tasks, evidence, comments and review history are preserved; FY/AY metadata and due-date calculation are synchronised.

+
+ Current FY {{ row.financial_year }} +
+
+ +
+ + +
+
+ + +
+
+ +
+
+

Locked/closed engagements, locked FYs, released final documents/UDINs and duplicate target engagements are blocked from correction.

+
+ {% endif %} + {% if row.is_locked %}
This engagement is locked as historical record. It cannot be edited.
{% endif %}

Client & Service

Client
{{ row.client.client_name if row.client else '-' }}
Service
{{ row.catalogue.service_name if row.catalogue else '-' }}
Financial Year
{{ row.financial_year or '-' }}
Assessment Year
{{ row.assessment_year or '-' }}
Return / Engagement Period
{{ row.period_label or 'Not applicable' }}
Engagement Type
{{ 'Assurance' if row.engagement_type == 'assurance' else 'Non-Assurance' }}
Original Due Date
{{ row.original_due_date or '-' }}
Expiry Date
{{ row.expiry_date or '-' }}
Current Due Date
{{ row.current_due_date or '-' }}{% if row.due_date_source %}{{ row.due_date_source|replace('_',' ')|title }}{% endif %}
Status
{{ 'Locked' if row.is_locked else row.status|replace('_',' ')|title }}{% if not row.is_active %} / Inactive{% endif %}
Engagement Dates
{{ row.start_date or '-' }} to {{ row.end_date or '-' }}
diff --git a/app/modules/services/templates/services/engagements/list.html b/app/modules/services/templates/services/engagements/list.html index 00134ea..a7733ba 100644 --- a/app/modules/services/templates/services/engagements/list.html +++ b/app/modules/services/templates/services/engagements/list.html @@ -28,6 +28,14 @@
{% endif %} + {% if fy_corrected_count or fy_skipped_count or fy_error %} +
+ {% if fy_corrected_count %}{{ fy_corrected_count }} engagement{{ 's' if fy_corrected_count != 1 else '' }} moved to the corrected financial year.{% endif %} + {% if fy_skipped_count %}{{ fy_skipped_count }} selected engagement{{ 's' if fy_skipped_count != 1 else '' }} skipped.{% endif %} + {% if fy_error %}{{ fy_error }}{% endif %} +
+ {% endif %} + {% if locked_count or skipped_count %}
{% if locked_count %}{{ locked_count }} engagement{{ 's' if locked_count != 1 else '' }} locked.{% endif %} @@ -51,15 +59,35 @@ -
+ {% if include_inactive %}{% endif %} - {% if can_lock_engagements %} -
-

Select completed engagements and lock them in bulk. Locked engagements become read-only history.

- + {% if can_lock_engagements or can_correct_engagement_fy %} +
+ {% if can_lock_engagements %} +
+

Select engagements below. Completed engagements can be locked as read-only history.

+ +
+ {% endif %} + {% if can_correct_engagement_fy %} +
+
+
+ + +
+
+ + +
+ +
+

Partner/Firm Admin only. Only open, active, unlocked engagements are changed. Duplicate target engagements and locked years are skipped.

+
+ {% endif %}
{% endif %} @@ -67,7 +95,7 @@ - {% if can_lock_engagements %}{% endif %} + {% if can_lock_engagements %}{% endif %} @@ -81,7 +109,7 @@ {% for row in rows %} - {% if can_lock_engagements %}{% endif %} + {% if can_lock_engagements %}{% endif %}
Client Service FY / AY
{% if not row.is_locked %}{% endif %}{% if not row.is_locked %}{% endif %}
{{ row.client.client_name if row.client else '-' }}
{{ row.client.client_code if row.client else '' }}
{{ row.catalogue.service_name if row.catalogue else '-' }}
{{ row.catalogue.service_code if row.catalogue else '' }}
FY: {{ row.financial_year or '-' }}
AY: {{ row.assessment_year or '-' }}
Period: {{ row.period_label or '-' }}