Design of Order, Shipment, PoD line item use of Orderable versioning

Hello everyone,

As we want to add orderable’s versioning to OpenLMIS, I’m currently working on the design of using it in order line item, shipment line item and proof of delivery line item. I went through our code and it looks like we don’t need many changes to achieve what we want.

Definitely, we’ll need to create a new class with orderable ID and version to group orderable info and avoid code duplication. Then we’ll be able to replace those properties with the newly created class object in the domain classes of mentioned line items so order, shipment, and proof of delivery line items will have a reference to the specific orderable version. The values of orderableId and orderedQuantity properties from OrderLineItem are set based on RequisitionLineItem fields and values in ShipmentLineItem are set based on OrderLineItem. Also, RequisitionLineItem should already have a reference to versioned orderable (orderable ID + version). This will be designed in OLMIS-3931.

It looks like none of the properties in line items are snapshotted besides one - useVvm parameter from ProofOfDeliveryLineItem. It is injected from OrderableDto’s extraData parameter in ShipmentService. It should work properly without any changes - the OrderableDto will have the proper version of orderable because it will be retrieved from referencedata service using the version from the PoD line item. Despite that, I’d suggest dropping useVvm parameter since we want to get rid of snapshotting in requisition and PoD will have reference to versioned orderable.

Furthermore, we’ll need to update some of our reports. Printing Proof of Delivery uses OrderableDto so it’s safe but printing orders and Pick pack list use SQL to generate a report. I think that the only change we need to apply is updating JOIN to check whether orderable version matches to the orderable version in PoD.

Any thoughts? Do you see any potential issues that I didn’t notice?

Regards,
Klaudia

Hi Klaudia,

from my point of view, the proposed changes make perfect sense. As a tester, I think that the only additional thing that needs to be checked is that whether exporting orders to CSV files, which is possible on the “View Orders” screen, will still work when these changes are applied. Apart from that, I don’t see any other potential issues, everything looks fine to me.

Regards,
Joanna

Thanks @Klaudia_Palkowska,

I think I follow, and your plan makes sense. One thing I’d note is that when you design a class for containing a reference to an Orderable with it’s identity, this is our first opportunity to establish the pattern we’d like to see in our API when we reference a versioned entity. I’d like us to follow the style from FHIR, though simplified.

e.g. as a relative path

...
"orderable": {
  "reference": "/api/orderables/5437bd1c-d582-4d3f-a7ce-8885c6d785b4?versionid=1"
}
...

Which we should add to our conventions.

Does anyone have thoughts on establishing this pattern? Does more need to be ironed out? Last thing I’d like to see is that we start having multiple different ways to reference and lookup a versioned entity, lets choose one now and stick with it until we need to change it.

Best,
Josh

Hi @joshzamor,

as I said on the technical committee meeting, we already have a pattern which is used in several places even in the fulfillment service: Object Reference DTO

In the following pattern, there are two fields id and href. The id field contains the resource id value and the href field contains the absolute URL to the resource.

Sometimes we extend this pattern with additional fields like username in order:
Order DTO
User Object Reference DTO

From what I know we are not using the href field to retrieve needed data on UI but rather we collect values from the id field and then we create a single request which return all required data.

In this case, I would say we should add additional field versionId and also the absolute URL should contain it. I am not sure if we should provide another pattern for our system.

Regards,
Lukasz

Thank you so much for your replies,

@Jbebak of course, we should verify whether every report related to orderables in the fulfillment service works as expected.

@llewczynski So you are you saying that we should use ObjectReference extended by versionId (and include it in the href) instead of adding a new class? I think it’s a good idea and doesn’t require establishing a new pattern.

@joshzamor and everyone else - what do you think? Any objections?

Best,
Klaudia

Thanks all for this, and yes lets use/extend what @llewczynski pointed us to.

One thing I’d ask: lets fill the documentation gap with these three patterns:

  1. the pattern that in referencing a Resource from another that we use
    "resourceName": {
        "href": "/api/resourceName",
        "id": "resourceUUID"
    } 
    
  2. the pattern is expanded when including a version
    "resourceName": {
        "href": "/api/resourceName",
        "id": "resourceUUID",
        "versionId": "sequentialId"
    } 
    
  3. that any versioned resource, such as Orderable and FTAP, is available at GET /api/resourceName?versionId=<sequentialVersionId>

I think that’s what we’re agreeing on. If I missed something please correct me. We should have this updated in our Service Style Guide.

Best,
Josh

As soon as I put that I noticed the Service Style Guide has this guideline:

When giving names to resources in the APIs, if it is a UUID, its name should have a suffix of “Id” to show that. (e.g. /api/users/{userId}/fulfillmentFacilities has query parameter rightId to get by right UUID.)

Using ID feels overloaded here as our Resources are always identified primarily by UUID, however versions of any particular Resource are always as a sequential id. If I see id as part of a field name or as a query parameter, that guideline tells me to always expect it to be a UUID.

Thoughts?

@joshzamor Sorry for the late response.

I don’t think you missed anything. I’ll create a ticket to update the documentation. For your latest post - you mean that also that part of Service Style Guide should be updated to avoid confusion with versionId?

Best,
Klaudia