OBPIH-5793 Reload button issue in bin replenishment

[alannadolny]

This issue was hard to spot like any other issue caused by the OSIV. After clicking the "refresh" button on the frontend side, this happened on the backend:
clear picklist -> refresh product availability -> create picklist
When we are creating a picklist we are getting available and suggested items. Suggested items are based on available items and additionally, they have to meet the following conditions:

availableItems?.findAll { it.quantityAvailable > 0 && it.pickable }

but each item had a quantity available equal to 0.
Available items are calculated using this query:

ProductAvailability.executeQuery("""
					select 
				        ii,
					pa.binLocation,
					pa.quantityOnHand,
				        pa.quantityAvailableToPromise
				from ProductAvailability pa
				left outer join pa.inventoryItem ii
				left outer join pa.binLocation bl
				where pa.location = :location
			""" +
                 "${excludeNegativeQuantity ? "and pa.quantityOnHand > 0" : ""}" +
                 "and pa.product.id in (:products)", [location: location, products: productsIds])

and here is the main point - when this query is executed, items that were picked to the picklist are still "picked" and their quantity is equal to 0. When we are executing this query the quantity should be greater than 0 for each item that we previously had in the picklist (even though we can change the quantity). Those quantities are 0 because we didn't commit those changes to the database, and creating and clearing picklist methods are separately called transactions that don't know about each other.

This issue could be solved by using flush: true when saving the picklist, but I remember that we tried to avoid this kind of change and I decided to spend more time, working on other solution, and I decided to move the refresh product availability method call to the controller (I left the same behavior that was previously in the clearPicklist method, so it should be working in other similar cases.)

[kchelstowski]

To extend the topic, since we've spent some time together of figuring that one out.
The problem is that this call:

openboxes/grails-app/services/org/pih/warehouse/inventory/ProductAvailabilityService.groovy

Lines 292 to 331 in 9d783dc

	boolean saveProductAvailability(Location location, Product product, List binLocations, Boolean forceRefresh) {
	log.info "Saving product availability for product=${product?.productCode}, location=${location}"
	def batchSize = ConfigurationHolder.config.openboxes.inventorySnapshot.batchSize ?: 1000
	def startTime = System.currentTimeMillis()
	try {
	Sql sql = new Sql(dataSource)
	// Execute SQL in batches
	// FIXME recover from deadlocks
	sql.withBatch(batchSize) { BatchingStatementWrapper stmt ->
	// If we need to force refresh then we want to set quantity on hand for all
	// matching records to 0.
	if (forceRefresh) {
	// Allows the SQL IFNULL to work properly
	// IFNULL(null, product_id)
	// IFNULL('10003', product_id)
	String productId = product?.id?"'${product?.id}'":null
	String forceRefreshStatement = String.format(
	"delete from product_availability " +
	"where location_id = '${location.id}' " +
	"and product_id = IFNULL(%s, product_id);", productId)
	stmt.addBatch(forceRefreshStatement)
	}
	binLocations.eachWithIndex { Map binLocationEntry, index ->
	String insertStatement = generateInsertStatement(location, binLocationEntry)
	stmt.addBatch(insertStatement)
	}
	stmt.executeBatch()
	}
	log.info "Saved ${binLocations?.size()} records for location ${location} in ${System.currentTimeMillis() - startTime}ms"
	} catch (Exception e) {
	log.error("Error executing batch update for ${location.name}: " + e.message, e)
	publishEvent(new ApplicationExceptionEvent(e, location))
	return false
	}
	return true
	}

the sql query that uses new Sql(dataSource) "doesn't know" about the transaction where the picklist items were deleted here:

openboxes/grails-app/services/org/pih/warehouse/picklist/PicklistService.groovy

Lines 67 to 85 in 9d783dc

	void clearPicklist(OrderItem orderItem) {
	Picklist picklist = orderItem?.order?.picklist
	def binLocations = []
	if (picklist) {
	picklist.picklistItems.findAll {
	it.orderItem == orderItem
	}.toArray().each {
	it.disableRefresh = Boolean.TRUE
	picklist.removeFromPicklistItems(it)
	orderItem.removeFromPicklistItems(it)
	binLocations.add(it.binLocation?.id)
	it.delete()
	}
	picklist.save()
	}
	productAvailabilityService.refreshProductsAvailability(orderItem?.order?.origin?.id, [orderItem?.product?.id], binLocations?.unique(), false)
	}

The dataSource here just creates its own connection/pool and doesn't follow any changes that might be made throughout the request (but in different transactions).
So the only way to do it is either like Alan did or call the flush on delete of picklist item.

[jmiranda]

@alannadolny this is awesome. Two comments.

1. I thought OSIV was disabled in grails 3.

2. Can you please make sure this comment gets back into the ticket. I want our investigation and conclusion comments to go into the ticket and not the PR because we generally search Jira when we're trying figure out what's going on. The PRs are a black hole where knowledge goes to die.

cc @kchelstowski @awalkowiak @drodzewicz

[kchelstowski]

@jmiranda yes, OSIV is disabled in Grails 3. Alan meant that in Grails 1 it was working thanks/due to OSIV and in Grails 3 those "osiv issues" are the most difficult to diagnose, as it goes deeply into transactions/sessions lifecycle.

[jmiranda]

Let's go with the flush approach for now unless a better option presents itself.