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,42 @@ 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")
+ // TODO Test that this actually captures all of the validation errors on the input file
+ // 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, but
+ // you cannot add another command classes errors to a parent command class. Instead
+ // we probably need a custom validator for the picklistItems association on
+ // PicklistDataImportCommand and just call validate on the parent.
+ //
+ // TODO If nothing else, the code below should be cleaned up and added to a utility
+ // ErrorsUtil.copyAllErrors(source, destination)
  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 +263,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/AllocationService.groovy

@@ -0,0 +1,8 @@
+package org.pih.warehouse.inventory
+
+class AllocationService {
+
+
+
+
+}

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. We should throw an exception if we don't have a good answer.
+ // 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,82 @@ 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)
+
+ // TODO It's probably ok to use proudct availability data here. However, we
+ // probably want to validate against actual inventory items (calculated from
+ // transactions) before the stock movement is shipped (i.e. 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
+
+ // TODO Need to test whether this satisfies the requirement when passing a NULL
+ // bin location. We also probably want to deal with the case where the provided
+ // bin location is provided, but not found.
+
+ // Let's try to determine whether there's a specific available inventory item
+ // associated with the data provided by the user
+
+ AvailableItem availableItem = pickPageItem.getAvailableItem(inventoryItem, internalLocation)
+
+ // FIXME Need to move the following code to an allocation service.
+ // If there is no available item for the provided data, then we need to allocate an
+ // item based on the requirements specified in the ticket (OBPIH-6331).
+ if (!availableItem) {
+ Integer quantityRequired = data.quantity
+ List<AvailableItem> availableItems = pickPageItem.getAvailableItems(inventoryItem)
+ List<SuggestedItem> suggestedItems = getSuggestedItems(availableItems, quantityRequired)
+
+ // Scenario 1: All stock in default (no bin)
+ // TODO If there's enough available quantity in the default bin, then allocate
+ // quantity from the default location
+
+ // Scenario 2: All stock in one bin
+ // TODO If there's enough available quantity in one internal location, then
+ // then allocate stock from that location
+
+ // Scenario 3a: if lot has stock entries in receiving bins
+ // TODO Are receiving locations configured to be picking locations?
+ // TODO If there's available quantity in default + receiving locations, then
+ // allocate using FIFO algorithm. Need to check with Manon if this is what is
+ // expected.
+
+ // Scenario 3b: if lot has stock entries in default bin
+ // TODO If there's enough quantity available in default location, then
+ // allocate from default location.
+
+ // Scenario 3c: if lot has stock entries only in hold bins
+ // TODO If lot has stock entries only in hold bin, then throw an exception.
+
+ // Scenario 4: Any other scenario (stock in “real” bin and virtual bin, stock in multiple real bins)
+ // TODO If there's not enough available quantity in default + receiving
+ // locations, then throw an exception
+
+
+ }
+
+ // TODO At the end of this we need to determine if we have enough available quantity
+ // to actually allocate items. If not we should throw an exception.
+
+ // TODO The weird thing here is that we're doing this as a pre-allocation activity
+ // just to satisfy the validation requirements. We'll then need to do the same
+ // thing within the importPicklistItems method, so technically this code should
+ // be pulled out into its own method
+
+ // 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 +1267,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' : ''}`}