Remove one line of code

Nikodem Graczewski this is added to adjustment-creation.module.js. Did I miss anything?

Nikodem Graczewski this is added to adjustment-creation.module.js. Did I miss anything?

Address code review feedbacks.

OLMIS-6201: disallow choosing wrong unpack reason.
OLMIS-6201: disallow choosing wrong unpack reason.
OLMIS-6201: disallow choosing wrong unpack reason.

    • -0
    • +36
    /src/stock-unpack-kit/unpack-kit-constant.js
OLMIS-6199: Update Reason Categories to include Aggregation

Rename and reorganize validation methods

Validate one more condition (fail validation when more constituents were credited than number of kits unpacked.)

Resolve message keys that were not properly translating.

Update Readme to include service specific env varables.

Thanks Josh Zamor I appreciate the feedbacks.

Thanks Josh Zamor I appreciate the feedbacks.

Yes, these are coming from the env variable and I have also added those to the ref-distro too. The commit is added to this review.

Yes, these are coming from the env variable and I have also added those to the ref-distro too. The commit is added to this review.

Enum would be great. Are you thinking of something like "StockEventType" that would be an attribute of the StockEventDto? In general, My observation with the current design is that Stock Event typ...

Enum would be great. Are you thinking of something like "StockEventType" that would be an attribute of the StockEventDto?

In general, My observation with the current design is that Stock Event types are identified by guessing. If any line item has destination or source, then it must be transfer. If none of the line items have reason, then it must be physical inventory. Or something in those lines. I don't think this much guessing scales as more event types are introduced.

I think Introducing Stock Event Type as an Enum would be a significant breaking change that will potentially affect clients as well. My reason for going with another guess work (if there is one unpack reason, then it must be a kit unpacking event) for Kit Unpacking was to avoid that breaking change.

Since we are too close to the release 3.6, I do not think it would be ideal to break so much. One other thing that can be done for the short term and as a step towards using the Enum properly can be to introduce the Enum but initialize it using the existing guess methods when the processing context is set. Would that be okay?

It checks if the quantity supplied by the client for the constituents accounts for what was specified in the unpack list. Example: Assume the unpack list for Kit A says, A contains, 10 Bs and 12 C...

It checks if the quantity supplied by the client for the constituents accounts for what was specified in the unpack list.

Example: Assume the unpack list for Kit A says, A contains, 10 Bs and 12 Cs. Assume 2 As were unpacked. This method checks if there are two or more line items that account for 20 Bs and 24 Cs. If there are line items for 20Bs and 20Cs, it would throw an error because the 4Cs are not accounted for.

I will rename the method.

Ignore adjustment reason validation for kit unpacking

OLMIS-682: Update settings-sample.env to include environment variables for unpack reasons

OLMIS-682: Unpack kit into stock
OLMIS-682: Unpack kit into stock
Load unpack reason id in context builder class.

Updates to fix PMD complaints

OLMIS-682: Unpack kit into stock

- Introduce new Reason Category (AGGREGATION)

- Add 2 new reasons for Kit unpacking and unpacked from kit.

- Add configuration that would be read from environment variable to determine which reason triggers adjustments.

- Implement an unpack kit validator that validates if all constituents from unpacked kit are accounted for

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.