Orders are not correctly voided when an Encounter is voided

The OrderService.voidOrder method has some custom business logic baked into it to address the fact that voiding an order may have an impact on any "previousOrder" that it closed.  Specifically it has this logic - see OrderServiceImpl:voidOrder(Order, String):

Order previousOrder = order.getPreviousOrder();
if (previousOrder != null && isDiscontinueOrReviseOrder(order)) {
   setProperty(previousOrder, "dateStopped", null);
}

When operating on Orders directly, this works as intended.

The authors of the EncounterService.voidEncounter(Encounter, String) clearly saw the need to delegate down to the service methods for nested objects, as there is this logic in place in that method:

ObsService os = Context.getObsService();
for (Obs o : encounter.getObsAtTopLevel(false)) {
   if (!o.getVoided()) {
      os.voidObs(o, reason);
   }
}

OrderService orderService = Context.getOrderService();
for (Order o : encounter.getOrders()) {
   if (!o.getVoided()) {
      orderService.voidOrder(o, reason);
   }
}

However, in practice, we find that when an Encounter is voided, neither of the ObsService.voidObs or OrderService.voidOrder methods are ever getting called, and although the associated Orders end up voided, the logic responsible for "un-stopping" previous orders is never executed.

The reason for this is that our AOP-based "RequireDataAdvice" executes prior to the method execution of EncounterService.voidEncounter, and pre-emptively and recusively voids all nested properties of Encounter that implement Voidable.  So by the time the above method is executed, all of the Obs or Orders are voided, and the voided check ensures that the associated service method call is never invoked.

The simplest solution would be to remove the voided check around both Obs and Orders, and call the void method on each regardless of whether the object is already voided or not.  We can tweak the logic of the downstream voidXyz service methods to not update the voidedBy and dateVoided (and possibly voidReason) properties if they are already populated, if that is the reason why this check exists.

Other solutions welcome - dkayiwa, mogoodrich, ibacher, wyclif  let me know if there are any thoughts or concerns.  ddesimone FYI.