Design Requisition Line Item use of Orderable/FTAP versioning

Hi everyone,

I’m currently designing an architecture of using Orderable and FTAP versioning in Requisition Line Item (RLI). I have figured out the following changes which should be committed in RLI:

  1. Adding FTAP id and FTAP version to RLI - to reference an FTAP in RLI to handle multiple version
  2. Adding Orderable version to RLI - to reference an Orderable in RLI to handle multiple version. Orderable id has been already added.
  3. Removing maxPeriodsOfStack from RLI entity - this field is currently snapshotted (copied) from FTAP. After adding a reference to FTAP, the FTAP fields shouldn’t be stored in RLI but retrieved from FTAP on demand.
  4. Removing packsToShip, pricePerPack, totalCost and nonFullSuply from RLI entity - the fields are currently snapshotted (fetched from Orderable and calculated). The values should be retrieved via Orderable reference.
  5. Id and version should be encapsulated in one class eg. EntityReference. It should apply both to Orderable and FTAP reference.

There should be a migration committed for the existing RequisitionLineItems. The simplest approach is to drop the mentioned fields and obtain the values by referencing the latest available Orderable/FTAP version.

What do you think about the described solution? Do you have any suggestions or do you see any potential impediments?

1 Like

Thanks Paweł, this sounds good to me.

Similar to what Klaudia stated in her post, we will need to review and see what work is needed in the requisitions reports (I’m thinking about requisition print - do we have any other?). The reports should also use versioned orderables.

It’s worth noting that this will require a cross-service migration (we need data both from requisitions and referencedata). Using latest orderable/FTAP version is a big assumption, but since the requisition service never fully supported updating products, I think this is fine - we will need to document this in the release notes for sure though.

Best,
Sebastian

Thank you Sebastian for your reply.

I have found only one requisition report which uses orderable fields - RequisitionLines.jrxml (link). In contrast to Proof of Delivery report we do not use SQL query there. Orderables are retrieved by the service on the backend, so if we committed the changes to the RequisitionLineItem and related classes, the report should have access to orderable’s fields automatically.

However, we plan to remove some parameters (eg. pricePerPack) so there will be a need to replace these fields with the proper methods which use up-to-date orderable state (eg. calculatePricePerPack()).

Best,

Thanks @pcieszko, and I agree with @Sebastian_Brudzinski.

I’d highlight two more things that might be effected by this breaking change:

  • the reporting flows in Nifi
  • the OpenSRP integration

Since this is a breaking API change to Requisitions I’m wondering if we might want to continue getting used to staging the change: make all the changes on our side and switch over, but keep the snapshotted information in the API response for a release cycle or two. Or we could version GET Requisition like we did with /api/v2/stockCardSummaries (i.e. add a new resource under /api/v2). Either way it’d be good to keep practicing the habit of limiting our breaking API changes and giving grace periods for external systems.

Thoughts?

Best,
Josh

Thanks @joshzamor for your remarks.

We think about adding the separated endpoints (/v2/) with the newest form of RequisitionLineItem. However, the previous form would be also supported. We’d like to implement /v2/ version step by step (starting at UI). This approach will let us avoid compatibility issues and that’s why we don’t need to worry about NiFi and OpenSRP so far. Nevertheless, the main disadvantage is that we will need to maintain more endpoints in code.

The following endpoints for “/requisitions” will be implemented in /v2/ form, because the planned changes in RequisitionLineItem would affect them:

  • BatchRequisitionController - retrieveAll, updateAll, saveAll
  • RequisitionController - initiate, updateRequisition, getRequisition, getSubmittedRequisitions

To sum up, seven new /v2/ endpoints will be created.

Best,
Paweł

Seven more endpoints is quite a number to maintain. I wonder if we need to do this for batch endpoints. Our UI is most likely the only consumer of those endpoints, so maybe we could only maintain v1 and v2 of endpoints you mentioned under RequisitionController?

Best,
Sebastian

I think that’s a good idea. We can reimplement API usage on the frontend for the batch endpoints. In this case, we can maintain only four additional endpoints for RequisitionController. By the way, I have verified that NiFi doesn’t use batch endpoints (it uses api/requisition/search).

Do you have any other remarks or do you see any impediments?

Best regards,
Paweł

While we will working on v2 endpoints maybe it would be worth to think about removing full representation of orderables and return only object reference.

I know that we do that because of the offline feature on UI but maybe it would be worth to think how much effort would cost us to implement other solution in which orderables would be taken from the browser cache. Thanks that we could increase the performance of endpoints.

@ngraczewski as a UI expert could you tell us what issues could cause that change?

Regards,
Lukasz

Hello everyone,

I think that the UI will require the following changes after removing those fields from the RequisitionLineItem class. :

  • ProductGridCell directive would have to be updated to also accept FTAP/orderable and search for values there
  • Product grid state would have to be updated to also fetch the FTAPS and orderables
  • FTAPS and orderables would have to be cached for each locally stored requisition in order for the offline to work.

Those are the things I can think of at the moment. Nothing else comes to my mind.

Best regards,
Nikodem

I’d say our UI caching pattern is already well-established to move forward with this, given that versioning of orderables and FTAPs is nearly done. We also need to retain the performance of the requisition endpoints after switching to versioned FTAPs, and this seems like a way to achieve this, and likely even further improve it.

To sum up, I think we should:

  • Introduce v2 for the four endpoints mentioned above (excluding batch approval endpoints)
  • Make v2 endpoints return object reference to orderables and FTAPs
  • Start work on caching orderables and FTAPs as soon as possible
  • Shift UI to use v2 when all of the above is complete

Does anyone see any concerns with this? cc @joshzamor

Best,
Sebastian

@Sebastian_Brudzinski I disagree on the part that we have a well-established pattern for UI caching. I have a feeling it’s all over the place. A different approach is used for caching requisitions, different for things cached during login.

I’d say we have some building blocks ready (LocalDatabase and post login actions), but we lack a strict architecture for using them. There is a lot of code duplication in places where we are caching things. We don’t have any strategy for updating data in the cache other than reloading or logging out and logging back in.

Personally, I’d take a step back and think about establishing some clean and dry architecture before we proceed with updating the UI.

Best regards,
Nikodem

2 Likes

Thanks @ngraczewski

Would you be able to create the necessary tickets for this work before the next backlog grooming? Sounds like a perfect tech debt for us to look into.

Best,
Sebastian

@Sebastian_Brudzinski

I’ve created a stub for it. It will require more discussion, definitely.

https://openlmis.atlassian.net/browse/OLMIS-6405

Best regards,
Nikodem

Thanks @ngraczewski, this is exactly what I’ve been worried about.