Fix/Fixed Display and Reachability of lines after admin decision

app/templates/budget/_work_item_lines_table.html

@@ -11,8 +11,25 @@ <h3 style="margin:0;">Budget Lines</h3>
   {% endif %}
 </div>
 
+{# A board-held budget is persisted as FINALIZED, so FINALIZED covers both
+   "finalized" and "Pending FY Budget Approval". The line page stays reachable
+   there because it renders read-only: every decision form on it requires a
+   checkout, and FINALIZED items cannot be checked out. #}
+{% set show_line_actions = status in ("SUBMITTED", "UNDER_REVIEW", "NEEDS_INFO", "FINALIZED") %}
+{% set line_action_label = "View" if status == "FINALIZED" else "Review" %}
+
 {% if lines %}
-  {% set show_approved_column = status == "FINALIZED" or totals.approved > 0 %}
+  {# Show the Approved column once an admin has decided any line, not once the
+     approved total goes positive. A rejection and a $0 approval are both real
+     decisions worth 0 cents; gating on the amount hid the column on exactly the
+     requests that needed it, leaving a rejected line's requested amount as the
+     only figure on the page. Comped badges and third-party-pays rooms make $0
+     approvals routine, not an edge case. #}
+  {% set has_admin_decision = lines
+       | selectattr("current_review_stage", "equalto", "ADMIN_FINAL")
+       | list | length > 0 %}
+  {% set show_approved_column = status == "FINALIZED"
+       or has_admin_decision or totals.approved > 0 %}
   <table class="responsive-cards">
     <thead>
       <tr>
@@ -27,7 +44,7 @@ <h3 style="margin:0;">Budget Lines</h3>
           <th style="width: 120px;" class="right">Approved</th>
         {% endif %}
         <th style="width: 120px;">Status</th>
-        {% if status == "SUBMITTED" or status == "UNDER_REVIEW" or status == "NEEDS_INFO" %}
+        {% if show_line_actions %}
           <th style="width: 80px;"></th>
         {% endif %}
       </tr>
@@ -123,10 +140,10 @@ <h3 style="margin:0;">Budget Lines</h3>
               {{ render_status_pill(line_status, label=friendly_status(line_status) if friendly_status else line_status) }}
             {% endif %}
           </td>
-          {% if status == "SUBMITTED" or status == "UNDER_REVIEW" or status == "NEEDS_INFO" %}
+          {% if show_line_actions %}
             <td>
               <a class="btn btn-muted" href="{{ url_for('approvals.line_review', event=ctx.event_cycle.code, dept=ctx.department.code, public_id=work_item.public_id, line_num=line.line_number) }}" style="padding: 4px 8px; font-size: 12px;">
-                Review
+                {{ line_action_label }}
               </a>
             </td>
           {% endif %}
@@ -143,7 +160,7 @@ <h3 style="margin:0;">Budget Lines</h3>
               <td class="right num">{{ format_currency(g.approved_cents) }}</td>
             {% endif %}
             <td></td>
-            {% if status == "SUBMITTED" or status == "UNDER_REVIEW" or status == "NEEDS_INFO" %}
+            {% if show_line_actions %}
               <td></td>
             {% endif %}
           </tr>
@@ -156,7 +173,7 @@ <h3 style="margin:0;">Budget Lines</h3>
               <td class="right num" style="color: #059669;">{{ format_currency(visible_approved_cents) }}</td>
             {% endif %}
             <td></td>
-            {% if status == "SUBMITTED" or status == "UNDER_REVIEW" or status == "NEEDS_INFO" %}
+            {% if show_line_actions %}
               <td></td>
             {% endif %}
           </tr>
@@ -175,7 +192,7 @@ <h3 style="margin:0;">Budget Lines</h3>
           <td class="right num" style="color: #059669;">{{ format_currency(totals.approved) }}</td>
         {% endif %}
         <td></td>
-        {% if status == "SUBMITTED" or status == "UNDER_REVIEW" or status == "NEEDS_INFO" %}
+        {% if show_line_actions %}
           <td></td>
         {% endif %}
       </tr>
@@ -188,7 +205,7 @@ <h3 style="margin:0;">Budget Lines</h3>
             {% if total_diff > 0 %}+{% endif %}{{ format_currency(total_diff) }}
           </td>
           <td></td>
-          {% if status == "SUBMITTED" or status == "UNDER_REVIEW" or status == "NEEDS_INFO" %}
+          {% if show_line_actions %}
             <td></td>
           {% endif %}
         </tr>

app/templates/budget/work_item_detail.html

@@ -117,8 +117,11 @@ <h1 style="margin:0;">
       </form>
     {% endif %}
 
-    {# Quick Review button for reviewers/admins (not shown for drafts) #}
-    {% if not perms.is_draft and (perms.is_worktype_admin or perms.can_checkout or perms.is_checked_out_by_current_user) %}
+    {# Quick Review button for reviewers/admins. Hidden for drafts, and for
+       FINALIZED items because it then repeats what the lines table already
+       shows; the per-line View link covers the read-only case. #}
+    {% if not perms.is_draft and status != "FINALIZED"
+          and (perms.is_worktype_admin or perms.can_checkout or perms.is_checked_out_by_current_user) %}
       <a class="btn btn-primary" href="{{ url_for('work.quick_review', event=ctx.event_cycle.code, dept=ctx.department.code, public_id=work_item.public_id) }}">
         Quick Review
       </a>

tests/integration/test_finalized_line_links.py

@@ -0,0 +1,111 @@
+"""Integration tests: per-line action link on a FINALIZED budget request.
+
+A board-held budget is persisted as FINALIZED; "Pending FY Budget Approval"
+is a derived display string. Both cases must still expose the line detail
+page, which renders read-only because every decision form requires a
+checkout and FINALIZED items cannot be checked out.
+"""
+import re
+from datetime import datetime
+
+from app import db
+from app.models import (
+    WorkItem, WorkLine, BudgetLineDetail,
+    REQUEST_KIND_PRIMARY,
+    WORK_ITEM_STATUS_FINALIZED, WORK_ITEM_STATUS_SUBMITTED,
+    WORK_LINE_STATUS_APPROVED, WORK_LINE_STATUS_PENDING,
+    REVIEW_STAGE_ADMIN_FINAL, REVIEW_STAGE_APPROVAL_GROUP,
+)
+
+DETAIL_URL = "/TST2026/TESTDEPT/budget/item/TST2026-TESTDEPT-BUD-1"
+
+# The per-line action cell, captured with its label so the test can tell
+# "Review" (actionable) from "View" (finalized) apart.
+LINE_ACTION = re.compile(
+    r'<a class="btn btn-muted" href="[^"]*/line/1/review"[^>]*>\s*(\w+)\s*</a>'
+)
+
+
+def _make_item(data, status, line_status, stage):
+    work_item = WorkItem(
+        portfolio_id=data["portfolio"].id,
+        request_kind=REQUEST_KIND_PRIMARY,
+        status=status,
+        public_id="TST2026-TESTDEPT-BUD-1",
+        created_by_user_id=data["admin"].id,
+    )
+    if status == WORK_ITEM_STATUS_FINALIZED:
+        work_item.finalized_at = datetime.utcnow()
+        work_item.finalized_by_user_id = data["admin"].id
+    db.session.add(work_item)
+    db.session.flush()
+
+    line = WorkLine(
+        work_item_id=work_item.id, line_number=1,
+        status=line_status, current_review_stage=stage,
+        approved_amount_cents=450_000 if line_status == WORK_LINE_STATUS_APPROVED else None,
+    )
+    db.session.add(line)
+    db.session.flush()
+    db.session.add(BudgetLineDetail(
+        work_line_id=line.id,
+        expense_account_id=data["expense_account"].id,
+        spend_type_id=data["spend_type"].id,
+        quantity=1, unit_price_cents=450_000,
+        routed_approval_group_id=data["approval_group"].id,
+    ))
+    db.session.commit()
+    return work_item
+
+
+def _finalized(data):
+    return _make_item(data, WORK_ITEM_STATUS_FINALIZED,
+                      WORK_LINE_STATUS_APPROVED, REVIEW_STAGE_ADMIN_FINAL)
+
+
+def _submitted(data):
+    return _make_item(data, WORK_ITEM_STATUS_SUBMITTED,
+                      WORK_LINE_STATUS_PENDING, REVIEW_STAGE_APPROVAL_GROUP)
+
+
+def _login(client, user_id):
+    with client.session_transaction() as sess:
+        sess["active_user_id"] = user_id
+
+
+def test_finalized_item_keeps_a_link_to_the_line(app, client, seed_workflow_data):
+    _finalized(seed_workflow_data)
+    _login(client, "test:admin")
+
+    resp = client.get(DETAIL_URL)
+    assert resp.status_code == 200
+    html = resp.get_data(as_text=True)
+
+    # The board hold is what the team reported against.
+    assert "Pending FY Budget Approval" in html
+    assert LINE_ACTION.search(html), "no per-line link on a finalized request"
+
+
+def test_finalized_line_link_is_labelled_view(app, client, seed_workflow_data):
+    _finalized(seed_workflow_data)
+    _login(client, "test:admin")
+
+    html = client.get(DETAIL_URL).get_data(as_text=True)
+    assert LINE_ACTION.search(html).group(1) == "View"
+
+
+def test_finalized_item_hides_quick_review(app, client, seed_workflow_data):
+    _finalized(seed_workflow_data)
+    _login(client, "test:admin")
+
+    html = client.get(DETAIL_URL).get_data(as_text=True)
+    assert "quick-review" not in html
+
+
+def test_submitted_item_still_shows_review_and_quick_review(app, client, seed_workflow_data):
+    _submitted(seed_workflow_data)
+    _login(client, "test:admin")
+
+    html = client.get(DETAIL_URL).get_data(as_text=True)
+    assert LINE_ACTION.search(html).group(1) == "Review"
+    assert "quick-review" in html

tests/integration/test_group_subtotals_view.py

@@ -4,7 +4,8 @@
     WorkItem, WorkLine, BudgetLineDetail, ApprovalGroup, UserRole,
     REQUEST_KIND_PRIMARY, WORK_ITEM_STATUS_SUBMITTED,
     WORK_LINE_STATUS_PENDING, WORK_LINE_STATUS_APPROVED,
-    REVIEW_STAGE_APPROVAL_GROUP, ROLE_APPROVER,
+    WORK_LINE_STATUS_REJECTED,
+    REVIEW_STAGE_APPROVAL_GROUP, REVIEW_STAGE_ADMIN_FINAL, ROLE_APPROVER,
 )
 
 
@@ -216,4 +217,54 @@ def test_approved_subtotals_render_when_approved_column_shown(app, client, seed_
 
     assert "Tech Team (" in html
     assert "Hotel Team (" in html
-    assert "$350.00" in html                     # approved subtotal for the TECH group
+    assert "$350.00" in html                     # approved subtotal for the TECH group
+
+def test_admin_final_rejection_shows_approved_column(app, client, seed_workflow_data):
+    """A rejected line needs the Approved column to show its $0.00.
+
+    Regression: the column gate tested totals.approved > 0, so an item whose
+    only admin decision was a rejection rendered requested amounts alone.
+    """
+    data = seed_workflow_data
+    tech = data["approval_group"]
+    work_item = _make_multi_group_item(data, [
+        (tech.id, 500_00, 1),   # 500.00 still pending
+        (tech.id, 76_00, 6),    # 456.00 rejected by the admin
+    ])
+
+    rejected = WorkLine.query.filter_by(
+        work_item_id=work_item.id, line_number=2,
+    ).first()
+    rejected.status = WORK_LINE_STATUS_REJECTED
+    rejected.current_review_stage = REVIEW_STAGE_ADMIN_FINAL
+    rejected.approved_amount_cents = None
+    db.session.commit()
+
+    with client.session_transaction() as sess:
+        sess["active_user_id"] = "test:admin"
+
+    resp = client.get("/TST2026/TESTDEPT/budget/item/TST2026-TESTDEPT-BUD-1")
+    assert resp.status_code == 200
+    html = resp.get_data(as_text=True)
+
+    assert ">Approved</th>" in html
+    assert "Difference:" in html
+
+
+def test_undecided_request_hides_approved_column(app, client, seed_workflow_data):
+    """No decisions yet means no Approved column; the gate must stay narrow."""
+    data = seed_workflow_data
+    tech = data["approval_group"]
+    _make_multi_group_item(data, [
+        (tech.id, 500_00, 1),
+        (tech.id, 76_00, 6),
+    ])
+
+    with client.session_transaction() as sess:
+        sess["active_user_id"] = "test:admin"
+
+    resp = client.get("/TST2026/TESTDEPT/budget/item/TST2026-TESTDEPT-BUD-1")
+    assert resp.status_code == 200
+    html = resp.get_data(as_text=True)
+
+    assert ">Approved</th>" not in html