Do we need to register a Custom Validator somewhere?

I guess I’m confused. Then why can’t RadiologyOrderValidator set the encounter property so it’s not null when OrderValidator does its validation?

Are validators not supposed to have side effects (i.e., change the object being validated)?

I’m a little confused too, if anyone wishes to do any custom validation before the core validator or set any fields like encounter that is straight forward invoke you validator manually before calling the API’s save method from your controller or servlet etc. Otherwise, currently the API provides no way to prioritize the order in which validators should be invoked.

OrderValidator has no order in the annotation but other validators do, for instance:

@Handler(supports = { Visit.class }, order = 50)
public class VisitValidator extends BaseCustomizableValidator implements Validator {

But HandlerUtil.getHandlersForType does not sort the validators by order.

Validation should be done the API; in this case, using a custom validator along with the built-in validation. Clients of the API should not have to manage special validation steps when working with RadiologyOrders; rather, clients should be able to treat RadiologyOrders like any other order and trust that the API will perform the proper business logic (including invoking any the custom validation associated with the order).

I don’t think we need tightly controlled sequencing, since >2 validators (more than core + a module-provided validator) for any given class would be an edge case. We just need to be able to guarantee that the core validation is performed last.

First off using a validator to mutate the object you are validating would be bad style. (E.g. I would certainly not expect that to be happening if I were reasoning about the behavior of the system.) @teleivo is not doing this, I just wanted to state it for the record. :slight_smile:

I would think about it like this:

  • The OpenMRS API requires all orders to belong to an encounter, and this is an intentional choice that we’re unlikely to change.
  • Therefore, if you want your module to hide this detail and let a developer make a one-line call that silently creates a new encounter and then saves the radiology order inside it, you need to write a convenience method for this.
  • It seems correct that you’d want to create the encounter and the order in a single db transaction, so the way you have done this (i.e. you put it in a Service) seems like the right approach (as opposed to having a Util method or something).
  • In your approach (e.g. where you hide encounters) you are envisioning a one-to-one mapping between radiology encounters and radiology orders. This is straightforward and “easier” but I don’t actually think it models the world quite right. If you’re expecting this to be used in a point-of-care system, where a clinician places radiology orders and also does other things, within the same clinical encounter, it might be better to approach this the way the encounter transaction web service in the emrapi module does. E.g. it’s (1) find an existing encounter, or create a new one if no suitable existing one is found, (2) add the order to that encounter.
1 Like