WIP: OBPIH-6331 Inferring bin locations in pick import

grails-app/controllers/org/pih/warehouse/picklist/PicklistController.groovy

@@ -10,6 +10,7 @@
 package org.pih.warehouse.picklist
 
 import grails.converters.JSON
+import grails.validation.ValidationException
 import org.pih.warehouse.api.PickPageItem
 import org.pih.warehouse.api.StockMovement
 import org.pih.warehouse.core.ActivityCode
@@ -218,26 +219,35 @@ class PicklistController {
  }
  }
 
- def importPickListItems(ImportDataCommand command) {
- Location location = command.location ?: Location.get(session.warehouse.id)
+ def importPickListItems(PicklistImportDataCommand command) {
 
- StockMovement stockMovement = stockMovementService.getStockMovement(params.id)
- List<PickPageItem> pickPageItems = stockMovementService.getPickPageItems(params.id, null, null)
+ // Bind stock movement based on provided id
+ command.stockMovement = stockMovementService.getStockMovement(params.id)
+
+ // Bind basic properties not provided by request
+ command.importType = "PicklistItems"
+ command.location = command.location ?: Location.get(session.warehouse.id)
 
- MultipartFile importFile = command.importFile
- if (importFile.isEmpty()) {
- throw new IllegalArgumentException("File cannot be empty")
+ // Validate provided properties of the data import, including the import file
+ if(!command.validate()) {
+ throw new ValidationException("Failed to import due to errors", command.errors)
  }
 
- String csv = new String(importFile.bytes)
- List<ImportPickCommand> importedLines = stockMovementService.parsePickCsvTemplateImport(csv)
+ // TODO There should probably be one driver method importPicklist() and all of the following
+ // should go into that method. The only things we need to do in the controller is validate
+ // the input and handle the errors.
+
+ // Bind the imported line items to the command class
+ command.picklistItems = stockMovementService.parsePicklistImport(command)
 
- Boolean supportsOverPick = location.supports(ActivityCode.ALLOW_OVERPICK)
- stockMovementService.validatePicklistListImport(pickPageItems, supportsOverPick, importedLines)
+ // Bind the parent requisition items to the command class
+ command.pickPageItems = stockMovementService.getPickPageItems(params.id, null, null)
 
+ stockMovementService.validatePicklistImport(command)
 
+ // TODO Errors should be added to a command class within the validate method
  List<String> errors = []
- importedLines.eachWithIndex { importPickCommand, index ->
+ command.picklistItems.eachWithIndex { importPickCommand, index ->
  if (importPickCommand.hasErrors()) {
  List<String> localizedErrors = importPickCommand.errors.allErrors.collect { g.message(error: it) }
  if (!localizedErrors.isEmpty()) {
@@ -246,12 +256,34 @@ class PicklistController {
  }
  }
 
- stockMovementService.importPicklistItems(stockMovement, pickPageItems, importedLines)
+ // FIXME Just pass the command object
+ stockMovementService.importPicklistItems(command.stockMovement, command.pickPageItems, command.picklistItems)
 
- if (!errors.isEmpty()) {
+ if (!command.hasErrors()) {
  render([message: "Data imported with errors", errors: errors] as JSON)
  }
 
  render([message: "Data imported successfully"] as JSON)
  }
 }
+
+
+class PicklistImportDataCommand extends ImportDataCommand {
+
+ Location location
+ StockMovement stockMovement
+ List<PickPageItem> pickPageItems
+ List<ImportPickCommand> picklistItems
+
+ PicklistImportDataCommand() {
+ importType = "picklistItems"
+ }
+
+ static constraints = {
+ // FIXME need to figure out if we can / want to bind stock movement and therefore make this nullable:false
+ stockMovement nullable: true
+ pickPageItems nullable: true
+ picklistItems nullable: true
+ }
+
+}

grails-app/domain/org/pih/warehouse/core/Location.groovy

@@ -285,6 +285,16 @@ class Location implements Comparable<Location>, java.io.Serializable {
  Location.list().findAll { it.isDepotWardOrPharmacy() }.sort { it.name }
  }
 
+ /**
+ * Find any internal location with the given name.
+ * @param name
+ * @return
+ */
+ List<Location> getInternalLocation(String name) {
+ // FIXME If location.name is not unique then we need to do some error handling
+ return locations.find { it.name.equalsIgnoreCase(name) }
+ }
+
  List<Location> getInternalLocations() {
  return locations?.toList()?.findAll { it.isInternalLocation() }
  }

grails-app/domain/org/pih/warehouse/product/Product.groovy

@@ -453,6 +453,36 @@ class Product implements Comparable, Serializable {
  }
  }
 
+ InventoryItem getDefaultInventoryItem() {
+ return getInventoryItem(null)
+ }
+
+
+ InventoryItem getInventoryItem(String lotNumber) {
+ return getInventoryItem(lotNumber, null)
+ }
+
+ // FIXME we could also just traverse the inventoryItems association rather than making an extra query
+ // FIXME but i wanted to move towards a detached criteria / named query fetch
+ // FIXME the expiration date should be involved here, but we don't require it in other places
+ // so i'll leave that for as a problem for future me
+ InventoryItem getInventoryItem(String lotNumber, Date expirationDate) {
+ InventoryItem.createCriteria().get() {
+ and {
+ eq("product", this)
+ if (lotNumber) {
+ eq("lotNumber", lotNumber)
+ } else {
+ or {
+ isNull("lotNumber")
+ eq("lotNumber", "")
+ }
+ }
+ }
+ }
+ }
+
+
  /**
  * Get ABC classification for this product at the given location.
  * @param locationId

grails-app/i18n/messages.properties

@@ -3365,7 +3365,8 @@ react.stockMovement.adjust.label=Adjust
 react.stockMovement.adjustStock.label=Adjust stock
 react.stockMovement.uploadDocuments.label=Upload documents
 react.stockMovement.printPicklist.label=Print picklist
-react.stockMovement.pickListItem.export.label=Export Pick
+react.stockMovement.importPicklist.label=Import Picklist
+react.stockMovement.exportPicklist.label=Export Picklist
 react.stockMovement.sortByBins.label=Sort by bins
 react.stockMovement.originalOrder.label=Original order
 react.stockMovement.sendShipment.label=Send shipment

grails-app/services/org/pih/warehouse/inventory/StockMovementService.groovy

@@ -50,6 +50,7 @@ import org.pih.warehouse.order.ShipOrderCommand
 import org.pih.warehouse.order.ShipOrderItemCommand
 import org.pih.warehouse.picklist.ImportPickCommand
 import org.pih.warehouse.picklist.Picklist
+import org.pih.warehouse.picklist.PicklistImportDataCommand
 import org.pih.warehouse.picklist.PicklistItem
 import org.pih.warehouse.product.Product
 import org.pih.warehouse.product.ProductAssociationTypeCode
@@ -1103,13 +1104,17 @@ class StockMovementService {
  return packPageItems
  }
 
- List<ImportPickCommand> parsePickCsvTemplateImport(String text) {
+ List<ImportPickCommand> parsePicklistImport(PicklistImportDataCommand command) {
+
+ // FIXME We should probably do some additional validation on the import file here or
+ // at least catch any exceptions that might happen
+ String text = new String(command.importFile.bytes)
  try {
  def csvMapReader = new CSVMapReader(new StringReader(text), [skipLines: 1])
  csvMapReader.fieldKeys = [
  'id',
- 'code',
- 'name',
+ 'code', // code => productCode
+ 'name', // name => productName
  'lotNumber',
  'expirationDate',
  'binLocation',
@@ -1132,15 +1137,38 @@ class StockMovementService {
  }
  }
 
- void validatePicklistListImport(List<PickPageItem> pickPageItems, Boolean supportsOverPick, List<ImportPickCommand> picklistItems) {
+ /**
+ * This method needs to be broken down into multiple smaller methods.
+ *
+ * @param command
+ */
+ void validatePicklistImport(PicklistImportDataCommand command) {
+
+ // The parent requisition items that picklist items are associated with
+ List<PickPageItem> pickPageItems = command.pickPageItems
+
+ // The allocated picklist items to validate
+ List<ImportPickCommand> picklistItems = command.picklistItems
+
+ // Configuration property that allows overpick for a given location
+ Boolean supportsOverpick = command.location.supports(ActivityCode.ALLOW_OVERPICK)
+
+ // TODO Do we handle the case where ID is null?
+ // Group the picklist items from the CSV by requisition item ID
  Map<String, List> groupedItems = picklistItems.groupBy { it.id }
 
  picklistItems?.each { ImportPickCommand data ->
+
+ // Base validation, creates errors object
  data.validate()
- // skip validation if id is empty since mos tof the validation relies on an existing requisition id
+
+ // TODO What happens if ID is null
+ // skip validation if id is empty since most of the validation relies on an existing requisition id
  if (!data.id) {
  return
  }
+
+
  PickPageItem pickPageItem = pickPageItems.find { it.requisitionItem?.id == data.id }
  if (!pickPageItem) {
  data.errors.rejectValue(
@@ -1151,9 +1179,30 @@ class StockMovementService {
  )
  }
  if (pickPageItem) {
- AvailableItem availableItem = pickPageItem.getAvailableItem(data.binLocation, data.lotNumber)
+
+ // Resolve given lot number to an inventory item
+ InventoryItem inventoryItem =
+ pickPageItem.requisitionItem.product.getInventoryItem(data.lotNumber, data.expirationDate)
+
+ // OBPIH-6331 Resolve bin location
+ // if given bin location is null or ambiguous then we need to do some special rules
+ Location internalLocation = command.location.getInternalLocation(data.binLocation)
+
+ // FIXME It's ok to use the available items if we ensure that we are validating against
+ // calculated inventory items before the stock movement is shipped / transactions created
+ // otherwise we could encounter a situation where the picklist item is valid based on
+ // the product availability, but this data might be stale for some reason
+ //AvailableItem availableItem = pickPageItem.getAvailableItems(inventoryItem)
+ AvailableItem availableItem = pickPageItem.getAvailableItem(inventoryItem, internalLocation)
+
+ // FIXME In a comment on ticket OBPIH-6331, Manon requested that we return
+ // multiple items based on certain conditions. This breaks the "contract" for
+ // the import and turns this from a manual import into an allocation algorithm
+ // but if it's actually required we can base the whole thing on allocation rules.
+ // List<SuggestedItem> suggestedItems = pickPageItem.getSuggestedItems(inventoryItem)
 
  if (!availableItem) {
+ // FIXME I don't like the way this is working but haven't had time to think of a better solution yet
  String lotNumberErrorMessage = data.lotNumber ?: "empty"
  String binLocationErrorMessage = data.binLocation ?: "empty"
  data.errors.rejectValue(
@@ -1166,7 +1215,7 @@ class StockMovementService {
 
  Integer itemQuantitySum = groupedItems[data.id].sum { it.quantity }
  if (itemQuantitySum > pickPageItem.requisitionItem.quantity) {
- if (!supportsOverPick) {
+ if (!supportsOverpick) {
  data.errors.rejectValue(
  "quantity",
  "importPickCommand.quantity.overpick.error",

src/js/components/stock-movement-wizard/outbound/PickPage.jsx

@@ -797,10 +797,10 @@ class PickPage extends Component {
  className="float-right mb-1 btn btn-outline-secondary align-self-end ml-1 btn-xs"
  >
  <span>
- <i className="fa fa-download pr-2" />
+ <i className="fa fa-upload pr-2" />
  <Translate
- id="react.default.button.importTemplate.label"
- defaultMessage="Import template"
+ id="react.stockMovement.importPicklist.label"
+ defaultMessage="Import Picklist"
  />
  </span>
  <input
@@ -821,7 +821,7 @@ class PickPage extends Component {
  className="float-right mb-1 btn btn-outline-secondary align-self-end ml-1 btn-xs"
  >
  <span>
- <i className="fa fa-upload pr-2" />
+ <i className="fa fa-download pr-2" />
  <Translate
  id="react.default.button.exportTemplate.label"
  defaultMessage="Export template"
@@ -833,7 +833,7 @@ class PickPage extends Component {
  onClick={() => this.exportPick(values)}
  className="float-right mb-1 btn btn-outline-secondary align-self-end ml-1 btn-xs"
  >
- <span><i className="fa fa-upload pr-2" /><Translate id="react.stockMovement.pickListItem.export.label" defaultMessage="Export Pick" /></span>
+ <span><i className="fa fa-download pr-2" /><Translate id="react.stockMovement.exportPicklist.label" defaultMessage="Export Pick" /></span>
  </button>
  <a
  href={`${this.state.printPicksUrl}${this.state.sorted ? '?sorted=true' : ''}`}

src/main/groovy/org/pih/warehouse/api/StockMovementItem.groovy

@@ -624,12 +624,53 @@ class PickPageItem {
  return availableItems ? availableItems?.sum { it.quantityAvailable } : null
  }
 
- AvailableItem getAvailableItem(String binLocationName, String lotNumber) {
- return availableItems?.find { item ->
- Boolean binLocationMatches = binLocationName ? item.binLocation?.name == binLocationName : !item.binLocation
- Boolean lotMatches = item.inventoryItem?.lotNumber == (lotNumber ?: null)
- binLocationMatches && lotMatches
+ /**
+ *
+ * @param inventoryItem
+ * @return
+ */
+ List<SuggestedItem> getSuggestedItems(InventoryItem inventoryItem) {
+
+ Integer quantityRequired = requisitionItem.quantity
+
+ List<AvailableItem> availableItems = getAvailableItems(inventoryItem)
+
+ // We've determined that bin location is null, so we need to suggest items from
+ // default or receiving bins, or throw an exception
+
+ // So I think we need to get the available items that match the requirements,
+ // sort them, then return the recommended amount
+
+ return new ArrayList<SuggestedItem>()
+ }
+
+ /**
+ * Get all available items for the given inventory item.
+ * @param inventoryItem
+ * @return
+ */
+ List<AvailableItem> getAvailableItems(InventoryItem inventoryItem) {
+ return availableItems.findAll { availableItem ->
+ return availableItem.inventoryItem == inventoryItem
+ }
+ }
+
+ /**
+ * Get all available items for the given inventory item and internal location.
+ * @param inventoryItem
+ * @param internalLocation
+ * @return
+ */
+ AvailableItem getAvailableItem(InventoryItem inventoryItem, Location internalLocation) {
+ // Return only exact matches on inventory item and internal location
+ return availableItems?.find { availableItem ->
+ return availableItem.inventoryItem == inventoryItem && availableItem.binLocation == internalLocation
  }
+// return availableItems?.find { item ->
+// Boolean binLocationMatches = binLocationName ? item.binLocation?.name == binLocationName : !item.binLocation
+// Boolean lotMatches = item.inventoryItem?.lotNumber == (lotNumber ?: null)
+// binLocationMatches && lotMatches
+// }
  }
 
  String getStatusCode() {