OBPIH-4749 Improvements to the stock movement list API#3498
OBPIH-4749 Improvements to the stock movement list API#3498awalkowiak merged 2 commits intofeature/ui-redesignfrom
Conversation
jmiranda
left a comment
jmiranda
left a comment
There was a problem hiding this comment.
There may be regressions added by this PR so test more than just the new list UIs.
|
|
||
| def stockMovements = stockMovementService.getStockMovements(stockMovement, [max: max, offset: offset, orderBy: orderBy ]) | ||
| if (params.format == 'csv' && stockMovements) { | ||
| def csv = getStockMovementsCsv(stockMovements) |
There was a problem hiding this comment.
I don't think we should be doing CSV from the API. Why not do this in the stock movement controller?
| } else { | ||
| def message = "No pending requisition items found" | ||
| response.status = 404 | ||
| render([errorMessage: message] as JSON) |
There was a problem hiding this comment.
If the client is expecting CSV, why are we returning errors as JSON?
| return csv | ||
| } | ||
|
|
||
| def getPendingRequisitionItemsCsv(List pendingRequisitionItems) { |
There was a problem hiding this comment.
Here, these are just requisition items. No need for the pending descriptor. What if I end up decided to pass a list of picked or issued requisition items. It doesn't seem to be doing anything special because of the "pending" status so it's probably not necessary.
| } | ||
| } | ||
|
|
||
| def getShipmentItemsCsv(List shipmentItems) { |
There was a problem hiding this comment.
In general, I don't like these methods here in the controller. They're better off in a service. I also think all of these could be more generalizable. We could convert each of these objects into a map and pass the map to one renderCsv method that does something very simple like this.
def keys = data[0].keySet()
csv.printRecord(keys)
data.each { row ->
csv.printRecord(row.values)
}
There was a problem hiding this comment.
And then all you need to do is convert the shipment or requisition item or whatever into a data map.
def data = shipments.collect {
return [
"Code" : shipmentItem.productCode,
"Product Name": shipmentItem.productName,
"Quantity Incoming": shipmentItem.quantity,
"Expected Shipping Date": shipmentItem.expectedShippingDate,
"Expected Delivery Date": shipmentItem.expectedDeliveryDate,
"Shipment Number": shipmentItem.shipmentNumber,
"Shipment Name": shipmentItem.shipmentName,
"Origin": shipmentItem.origin,
"Destination": shipmentItem.destination,
]
}
| shipmentItem.expectedDeliveryDate, | ||
| shipmentItem.shipmentNumber, | ||
| shipmentItem.shipmentName, | ||
| shipmentItem.origin, |
There was a problem hiding this comment.
I'm guessing the toString() is return the name, but you should be explicit here.
| shipmentItem.productCode, | ||
| shipmentItem.productName, | ||
| shipmentItem.quantity, | ||
| shipmentItem.expectedShippingDate, |
There was a problem hiding this comment.
Is there a preferred date format?
|
|
||
| try { | ||
| stockMovements = stockMovementService.getStockMovements(stockMovement, [max: max, offset: offset, createdAfter: createdAfter, createdBefore: createdBefore]) | ||
| stockMovements = stockMovementService.getStockMovements(stockMovement, params) |
There was a problem hiding this comment.
Doesn't look like createdAfter and createdBefore are added back to the params map above.
2 participants
No description provided.