Recently, we added the ability to set a supervisory node as a partner node for another supervisory node. This information is stored in the
extraData field in the
SupervisoryNode class. The partner node is a regular supervisory node and it is related to supply partner.
For now, we handle only single direction so regular node knows about a partner node but the partner node does not know that there is the regular node. We would like to add a missing information to the partner node - in other words, we would like to introduce a bidirectional relation between regular nodes and partner nodes.
As I said at the beginning we store the information in the
extraData field and because of that, some actions will be hard to maintain - mainly because we need to do this manually on service layer:
- if a user adds a new supervisory node that has some partner nodes, backend need to go through all existing nodes to add missing details.
- backend needs to do similar work when a supervisory node is removed from the system.
- the most difficult situation to handle is when we update a node because backend needs to update other nodes and also check if a node is not treated as a partner node by several regular nodes.
I am not 100% sure but I think this could be automatically handled by Hibernate if we would store information in the class field instead of in the
I agree that adding a separate fields could make things easier to maintain. Since the partner node model is a part of the reference data service I’m not sure it should be handled with a extraData field rather than it being a separate field of the Supervisory Node. In my mind, extraData property is great util for storing data provided by other services, but it comes with a cost of the harder maintenance and it has proven to be problematic in the past (example could reading VVM status on the UI screens). I’m not saying we should get rid of it completely, but we definitely should consider using a regular field instead every time we’re thinking about adding some property to the extraData.
I this case I agree that putting this data in the
extraData field seems incorrect. It seems like what we are doing here is changing the design Supervisory node data rather than adding some optional information to the node and, as such, I think that the corresponding model/table should be updated to reflect that change.
Given the nature of this change, all the newly required bi-directional data should be able to be computed and so this should not be a breaking change, correct?
I would say yes, because we only add new fields to Supervisory Node resource. We don’t change anything in the existing model structure.
Thanks for responses.
I decided to move partner node details from the
extraData field to normal model fields. I had to add two fields to the
partnerNodes - this is a collection that presents information about what partner nodes have the given node
partnerNodeOf - this is a field that presents information about which node see the given one as a partner node.
Also, because we want to make sure that a supervisory node can be seen as a partner node by only one supervisory node we were able to do this without creating an additional table on the database layer.
Apologies for the late reply (I had my email notifications tuned incorrectly and I wasn’t visiting every day).
As I’ve followed the general logic laid out here to move a partner node from the extraData fields to a first class field, I’ve generally nodded my head in agreement, on the surface of things it makes sense.
What I’m worried about is what does making a partner node a first class field in the API mean (to other services/developers)? I assume that a user’s permission strings and right’s aren’t effected by a supervisory node having a partner node, right? If I’m the CCE service, and I see a supervisory node has a partner node, however that doesn’t change what fridges that a CCE user can see, then what exactly do I interpret a partner node as?
The questions above are intended to be rhetorical. The worry I have with this change is that we’re taking the most complicated configuration topic in OpenLMIS, supervisory nodes, and making it more complex by adding partner nodes as first class API fields. I know I’m not seeing enough of a change to our documentation that matches the magnitude of making a supervisory node partner a first-class API concept. Are we going to tackle that documentation challenge? Or would we rather want to reconsider the approach here?