OFBIZ-11468 Improved: Convert ShipmentReceiptServices.xml mini lang to groovy #150
Conversation
(OFBIZ-11468) Also converted getTotalIssuedQuantityForOrderItem in IssuanceServices, because it is used in ShipmentReceiptServices and needed more return values.
|
Kudos, SonarCloud Quality Gate passed!
|
There was a problem hiding this comment.
There is no need to have subfolders defining where the services belong to when such is already captured in the name of the file, e.g.:
folder /shipment/receipt/ when the filename has ShipmentReceiptServices.groovy
Similarly
/shipment/issuance/ when the filename has ShipmentIssuanceServices.groovy
|
Hi Wiebke, After a break, I'll continue to review, sorry for the delay. I agree with Pierre. It does not make sense to have directories with a single file named "as" the directory, like it's in XML currently. |
|
Hi Wiebke, Would you have a chance to change as Pierre suggested? We though could do that later with another PR if your prefer... |
|
Hi, |
|
You later suggestion sounds good to me, Wiebke. I have still to finish the review, from what I saw so far (half of it), I'll maybe not do a complete review but just check another service before merging... |
|
Then I will leave the structure as it is and we can create a separate PR for it later |
| Boolean affectAccounting = true | ||
|
|
||
| GenericValue product = from("Product").where(parameters).queryOne() | ||
| if (product.productTypeId == "SERVICE_PRODUCT" || product.productTypeId == "ASSET_USAGE_OUT_IN" || product.productTypeId == "AGGREGATEDSERV_CONF") { |
There was a problem hiding this comment.
Though we use now 150 chars it would be easier to read if the or would be split by line.
To be sincere, this was more to test the GH review feature :)
| run service:"createInventoryItemDetail", with: inventoryItemDetailMap | ||
|
|
||
| // Balance the inventory item | ||
| Map balanceInventoryItemMap = [inventoryItemId: inventoryItem.inventoryItemId, priorityOrderId: shipmentReceipt.orderId, priorityOrderItemSeqId: shipmentReceipt.orderItemSeqId] |
There was a problem hiding this comment.
This line is 26 chars too long (180 instead of max 250)
| shipmentItem.quantity += quantityToAdd | ||
| shipmentItem.store() | ||
| GenericValue orderShipment = from("OrderShipment") | ||
| .where(orderId: parameters.orderId, orderItemSeqId: parameters.orderItemSeqId, shipmentId: parameters.shipmentId, shipmentItemSeqId: shipmentItem.shipmentItemSeqId) |
There was a problem hiding this comment.
another too long line
| * | ||
| * - for serialized items with a serial number passed in: the quantityAccepted _should_ always be 1 | ||
| * - if the type is SERIALIZED_INV_ITEM but there is not serial number (which is weird...) we'll create a bunch of individual InventoryItems | ||
| * - DEJ20070822: something to consider for the future: maybe instead of this funny looping maybe for serialized items we should only allow a quantity of 1, ie return an error if it is not 1 |
There was a problem hiding this comment.
Another (inherited) too long line
|
Very good code, just 4 too long lines, thanks for the good work Wiebke (even if I did not review all, I'm confident with what I saw about the code quality) |
|
Kudos, SonarCloud Quality Gate passed!
|
|
Hi Pierre, Wiebke, With 3926134 commit I have moved the files in up-dir |
|
Forgot to change in services def, Pawan handled it :) |
wpaetzold
deleted the
OFBIZ-11468_Convert_ShipmentReceiptServices-squashed
branch
(OFBIZ-11468)
Also converted getTotalIssuedQuantityForOrderItem in IssuanceServices, because it is used in ShipmentReceiptServices and needed more return values.