OLMIS-6062: put orderable/{id} increments version id.
OLMIS-6062: put orderable/{id} increments version id.
OLMIS-3933: add RAML entry for PUT /orderables/{id}
OLMIS-3933: add RAML entry for PUT /orderables/{id}
OLMIS-6062: put orderable/{id} increments version id.

Note: while the acceptance criteria for this ticket says this should be done on the repository layer

The current implementation is on the domain and may have to be updated in the future.

Josh Zamor and Chongsun Ahn FYI, I think resolving may require implementing something similar to: https://github.com/OpenLMIS/openlmis-referencedata/blob/ca3e4a0b3449fb09e7f72cbd6a7588cd99cc1db6/sr...

Josh Zamor and Chongsun Ahn FYI, I think resolving may require implementing something similar to: https://github.com/OpenLMIS/openlmis-referencedata/blob/ca3e4a0b3449fb09e7f72cbd6a7588cd99cc1db6/src/main/java/org/openlmis/referencedata/domain/RequisitionGroup.java#L174

Given, we plan to increment the version and persist the new version of orderables in the future, implementing something like the example above seems counter productive. For this reason, I went for a stop gap implementation that increments the version and gets me past this error. My implementation increments the version in the domain code (not on the repository) hope it is okay for now. I will create a separate review for what I have done.

OLMIS-3933: add RAML entry for PUT /orderables/{id}

    • -0
    • +21
    /src/main/resources/api-definition.yaml
This may be a defect then. I get the following error when I try to put an orderable with no change on program orderables. { "messageKey": "referenceData.error.orderable.programOrderable.duplica...

This may be a defect then. I get the following error when I try to put an orderable with no change on program orderables.

{
    "messageKey": "referenceData.error.orderable.programOrderable.duplicated",
    "message": "An orderable cannot have more than one active association to the same program."
}
Thanks. I will use this interpretation.

Thanks. I will use this interpretation.

In rest guideline, there is the following text. A PUT on a single resource (e.g. PUT /facilities/Unknown macro: {id} ) is not strictly an update; if the resource does not exist, one should be cr...

In rest guideline, there is the following text.


A PUT on a single resource (e.g. PUT /facilities/

Unknown macro: {id}

) is not strictly an update; if the resource does not exist, one should be created using the specified identity (assuming the identity is a valid UUID).

is this guideline optional? I think that check does not allow for creation of resources with specified id.

Does this handle one to many relationships well? Example, when orderables have programs, does it merge previously persisted programOrderables with the once in the dto properly? when the request has...

Does this handle one to many relationships well? Example, when orderables have programs, does it merge previously persisted programOrderables with the once in the dto properly? when the request has removed children from the one to many relationships, does this properly remove those?

OLMIS-683: Use @EqualsAndHashCode, rename orderableRef to orderable.

ah. I did not know about "of" in EqualsAndHashCode. Ok, I will go with ... of ={"parent", "orderable"}

ah. I did not know about "of" in EqualsAndHashCode. Ok, I will go with ... of =

Unknown macro: {"parent", "orderable"}
In most domain objects, business keys were used to determine equality. you can look at Orderables and ProgramOrderables objects for this pattern. I implemented business key based equality here to m...

In most domain objects, business keys were used to determine equality. you can look at Orderables and ProgramOrderables objects for this pattern. I implemented business key based equality here to make sure orderable child also follows the same pattern. @EqualsAndHashCode annotation does not necessarily implement business key based equality.

Ok. Let me try to change getOrderable to getOrderableId and see if the conflict goes away. Hope getOrderableId that returns UUID is okay by you.

Ok. Let me try to change getOrderable to getOrderableId and see if the conflict goes away. Hope getOrderableId that returns UUID is okay by you.

Unfortunately, We cannot have two fields with the same name but different datatype in the same class. Since the OrderableChild.Exporter and OrderableChild.Importer have getOrderable and setOrderabl...

Unfortunately, We cannot have two fields with the same name but different datatype in the same class. Since the OrderableChild.Exporter and OrderableChild.Importer have getOrderable and setOrderable, and the datatype for those had to be changed because of a previous feedback you gave me (not to have DTO on the importer/exporter declaration), I was forced to have the "ref" prefix on this property. I would appreciate if you can tell me any way around this.

OLMIS-683: Use Equals Verifier, Create Index

Thanks everyone for the review and feedbacks. It is clear that the minimal-dto pattern is an anti pattern and I have replaced it with the ObjectReferenceDto.

Thanks everyone for the review and feedbacks. It is clear that the minimal-dto pattern is an anti pattern and I have replaced it with the ObjectReferenceDto.

OLMIS-683: Remove minimal orderable dto

    • -2
    • +0
    /src/main/resources/api-definition.yaml
We need to have productCode and fullName attributes to be present for display. ObjectReferenceDto does not seem to support those attributes.

We need to have productCode and fullName attributes to be present for display. ObjectReferenceDto does not seem to support those attributes.

ObjectReferenceDto is a Final class and cannot be extended. Do we need to change that? By the way, MinialOrderableDto follows the same pattern used in MimimalFacilityDto. Does this mean MinimalFaci...

ObjectReferenceDto is a Final class and cannot be extended. Do we need to change that? By the way, MinialOrderableDto follows the same pattern used in MimimalFacilityDto. Does this mean MinimalFacilityDto would also need to extend ObjectReferencDto?