The job is not finished when the first visit is

A technician diagnoses a garage door on Tuesday, finds a snapped spring, orders the part, and packs up. He is done for the day and the job in his hand looks finished, so he marks it Completed.

That night the customer is invoiced for the diagnosis. On Thursday he comes back, fits the spring, and adds it to the job. Nothing bills. No error at any point — the invoice went out on Tuesday and it is never coming out again.

This is the failure the completion guard exists to prevent — with one condition, covered below: the Thursday visit has to exist as an appointment when he taps Completed. Most of the work was not the guard.

One job, two visits, one invoice

At Doorvana a service job is one WorkOrder, and each visit to the property is one ServiceAppointment on it. A diagnosis and a return trip for an ordered part are two appointments, one job, one invoice.

That shape is deliberate and it is the right one. It also means the billing trigger has exactly one moment to fire, and everything depends on that moment being the right one:

The guard is one sentence of policy: a WorkOrder cannot enter the Completed or Closed status category while any ServiceAppointment under it is still open. Everything below is what that sentence cost.

It has to run after update

WorkOrder.StatusCategory is not a field anybody sets. It is derived from Status through the WorkOrderStatus standard value set, and in a before-update trigger it still holds the old value. A guard that reads StatusCategory in before update is reading yesterday’s answer about today’s record.

So the check runs in after update, where StatusCategory already reflects the new Status, and it uses addError — which in after update rolls the entire transaction back, status change included.

private static final Set<String> CLOSED_WORK_ORDER_CATEGORIES =
    new Set<String>{ 'Completed', 'Closed' };

private static Map<Id, WorkOrder> selectClosing(
    List<WorkOrder> newWorkOrders, Map<Id, WorkOrder> oldWorkOrdersById
) {
    Map<Id, WorkOrder> closingById = new Map<Id, WorkOrder>();
    for (WorkOrder workOrder : newWorkOrders) {
        WorkOrder previous = oldWorkOrdersById.get(workOrder.Id);
        if (CLOSED_WORK_ORDER_CATEGORIES.contains(workOrder.StatusCategory)
            && !CLOSED_WORK_ORDER_CATEGORIES.contains(previous?.StatusCategory)) {
            closingById.put(workOrder.Id, workOrder);
        }
    }
    return closingById;
}

Only the transition counts. A Work Order that was already closed and gets edited is not closing again, and a history load that arrives already Completed was never a completion event at all — the same rule the billing trigger uses, for the same reason.

ParentRecordId is not only the Work Order

The obvious query is appointments where ParentRecordId is one of the Work Orders being completed. It is also wrong.

ServiceAppointment.ParentRecordId is polymorphic. It accepts WorkOrder, WorkOrderLineItem and Asset, and on a multi-door job it is entirely normal for an appointment to hang off a specific line item rather than the job as a whole. A guard that only knows about Work Order Ids sees no open appointments on a job that plainly has one.

So the guard builds a map from every possible parent back to its Work Order — the Work Order Ids themselves, plus the Ids of all their line items — and queries against the whole key set:

Map<Id, Id> workOrderIdByParentId = new Map<Id, Id>();
for (Id workOrderId : closingById.keySet()) {
    workOrderIdByParentId.put(workOrderId, workOrderId);
}
for (WorkOrderLineItem lineItem : WorkOrderCompletionSelector.selectLineItems(closingById.keySet())) {
    workOrderIdByParentId.put(lineItem.Id, lineItem.WorkOrderId);
}

One appointment, wherever it is parented, resolves back to one Work Order to put the error on.

The query has to see more than the user does

From API version 67.0 Apex runs in user mode by default, which on this project has already produced one post of its own. Here it bites from the other direction.

The user completing the job is a technician. The appointment that makes the job unfinished may be assigned to a different technician, and there is no guarantee the completing user is shared on that record. In user mode, that appointment simply is not in the result set — and an empty result set is indistinguishable from a finished job.

return [
    SELECT Id, AppointmentNumber, ParentRecordId
    FROM ServiceAppointment
    WHERE ParentRecordId IN :parentIds
        AND StatusCategory NOT IN :CLOSED_APPOINTMENT_CATEGORIES
    WITH SYSTEM_MODE
    ORDER BY AppointmentNumber
];

The selector reads Ids, the appointment number and the parent, and nothing leaves the transaction except the appointment number in the error message. Widening visibility in a guard is only defensible when the widened data does not escape.

Closed is quieter than Completed

Blocking Completed is the obvious half. Closed matters more than it looks.

Billing fires on the transition into Completed. A Work Order dragged straight from In Progress to Closed skips that transition entirely, so it is not misbilled — it is never billed. The loud failure is a half-invoice; the quiet one is no invoice, on a job that reads as finished on every list view in the org.

Both categories are in the set, and there is a test for each.

The status that had to exist first

Here is the part that is actually design rather than code: a guard that only says no is a guard that gets worked around. The technician on Tuesday is not being careless. He has finished what he can do today and the system offers him no honest way to say so.

So Awaiting Parts was added to the WorkOrderStatus standard value set, and the groupingString is the whole point:

<standardValue>
    <fullName>Awaiting Parts</fullName>
    <default>false</default>
    <label>Awaiting Parts</label>
    <groupingString>OnHold</groupingString>
</standardValue>

groupingString is what StatusCategory resolves to. Because the guard keys on the category and not on the status label, a status mapped to OnHold is automatically not a completion — no Apex change, no list of blessed status names to keep in sync. A future Awaiting Approval mapped the same way inherits the behaviour for free.

The error message then points at it, verbatim:

This job still has an open service appointment (SA-0042). Complete or cancel it first.
If the tech is coming back with a part, set the status to Awaiting Parts instead.

Naming the specific appointment matters because on a multi-door job the technician needs to know which visit is outstanding, and naming the alternative matters because otherwise the next thing tried is Closed.

Jobs parked in Awaiting Parts get their own list view, so a job waiting on a spring is a queue somebody works rather than a record that quietly ages. Deposit tracking lives on the same view: a deposit is a payment rather than revenue, so it never becomes an OrderItem — Salesforce records only that one is required and whether it has arrived, so nobody orders a part before it does. The Order stays at the full value of the job and the accounting system applies the deposit against the invoice.

What the guard cannot see

The guard reads appointments that exist. If the technician on Tuesday completes his own appointment and nobody has created Thursday’s yet, the query finds nothing open, the guard passes, and the Work Order moves into Completed and bills the diagnosis — exactly the opening scenario.

This limitation was recorded on the project after the guard shipped, while working out what “done” means for a two-visit repair. The guard is still worth having — it stops the case where the return visit is booked and someone closes the job anyway — but it is half of the control, not all of it.

What the tests pin down

Eight tests, and the useful thing is which cases they are:

• an open appointment on the WorkOrder blocks completion, and the status change is rolled back
• an open appointment on a line item blocks it too
• Closed is blocked the same way as Completed
• a blocked Work Order produces zero Orders
• in a bulk update, every Work Order with an open appointment is rejected individually
• Completed and Canceled appointments do not block, and the job bills exactly once
• moving to Awaiting Parts with an appointment open succeeds
• null trigger collections block nothing

The one that earns its place is shouldNotBill_WhenCompletionBlocked. addError in after update rolls the transaction back, so the billing that ran earlier in the same transaction is rolled back with it, and the trigger handler also excludes blocked Ids before enqueuing billing. Belt and braces — and the assertion is on the Order count, because that is the thing the customer sees.

The deploy validated with RunSpecifiedTests: 38 of 38 passing.

Practical summary

• Read WorkOrder.StatusCategory in after update, never before update — it is derived from Status and holds the old value until the update is applied.
• Guard on the transition into a closed category, not on the category itself, or every later edit to a finished job re-triggers everything.
• ServiceAppointment.ParentRecordId is polymorphic. Collect the Work Order’s line item Ids and query both, or you will miss appointments and pass a job that is not finished.
• A guard may legitimately need WITH SYSTEM_MODE to see records the running user cannot. Say so in a comment, and do not let the widened data leave the transaction.
• A guard on open appointments cannot see a visit nobody has created. Book the return appointment, or park the job in an on-hold status, before closing out the first visit.
• Block Closed as well as Completed. Skipping Completed skips billing altogether, which is the failure nobody notices.
• Add the alternative status before adding the block, map it with groupingString so category logic picks it up automatically, and give it a list view so parked jobs stay visible.

The pattern holds across everything on this build so far: the automation was the easy half. The hard half was making sure the person holding the tablet had a true thing they could click.