From 2351ad01e5698d1407f0ebf32b809dca8e67dff6 Mon Sep 17 00:00:00 2001 From: Walkowiak Date: Tue, 8 Jul 2025 11:17:07 +0200 Subject: [PATCH 1/2] OBPIH-7198 Add possibility to migrate old product inventory transactions --- grails-app/conf/runtime.groovy | 3 + .../warehouse/data/MigrationController.groovy | 30 +- .../warehouse/data/MigrationService.groovy | 289 +++++++++++++++++- ...nventoryTransactionMigrationService.groovy | 25 ++ .../ProductInventoryTransactionService.groovy | 29 +- grails-app/views/migration/dataMigration.gsp | 30 ++ .../org/pih/warehouse/core/Constants.groovy | 2 + 7 files changed, 398 insertions(+), 10 deletions(-) create mode 100644 grails-app/services/org/pih/warehouse/inventory/ProductInventoryTransactionMigrationService.groovy diff --git a/grails-app/conf/runtime.groovy b/grails-app/conf/runtime.groovy index 0eb56281223..564f72c4189 100644 --- a/grails-app/conf/runtime.groovy +++ b/grails-app/conf/runtime.groovy @@ -97,6 +97,9 @@ openboxes.transactions.inventoryBaseline.inventoryImport.enabled = true // Inventory snapshot configuration (OBPIH-7254) openboxes.transactions.inventoryBaseline.loadDemoData.enabled = true +// Inventory snapshot configuration (OBPIH-7198) +openboxes.transactions.inventoryBaseline.migration.enabled = true + openboxes.security.rbac.rules = [ [controller: '*', actions: ['delete'], accessRules: [ minimumRequiredRole: RoleType.ROLE_SUPERUSER ]], [controller: '*', actions: ['remove'], accessRules: [ minimumRequiredRole: RoleType.ROLE_SUPERUSER ]], diff --git a/grails-app/controllers/org/pih/warehouse/data/MigrationController.groovy b/grails-app/controllers/org/pih/warehouse/data/MigrationController.groovy index b5d7d93e0e5..0f9b9f84c6d 100644 --- a/grails-app/controllers/org/pih/warehouse/data/MigrationController.groovy +++ b/grails-app/controllers/org/pih/warehouse/data/MigrationController.groovy @@ -37,7 +37,6 @@ class MigrationController { def dataService def migrationService - def inventoryService def locationService def productAvailabilityService @@ -55,11 +54,19 @@ class MigrationController { def organizations = migrationService.getSuppliersForMigration() def productSuppliers = migrationService.getProductsForMigration() TransactionType inventoryTransactionType = TransactionType.load(Constants.INVENTORY_TRANSACTION_TYPE_ID) - def inventoryTransactionCount = Transaction.countByTransactionType(inventoryTransactionType) + TransactionType productInventoryTransactionType = TransactionType.load(Constants.PRODUCT_INVENTORY_TRANSACTION_TYPE_ID) + Integer inventoryTransactionCount = Transaction.countByTransactionType(inventoryTransactionType) + Integer productInventoryTransactionCount = Transaction.countByTransactionType(productInventoryTransactionType) + Location currentLocation = Location.get(session.warehouse.id) + Integer productInventoryTransactionInCurrentLocationCount = Transaction.countByTransactionTypeAndInventory(productInventoryTransactionType, currentLocation.inventory) + List productsWithProductInventoryTransactionInCurrentLocation = migrationService.getProductsWithTransactions(currentLocation, productInventoryTransactionType) [ organizationCount : organizations.size(), inventoryTransactionCount: inventoryTransactionCount, + productInventoryTransactionCount: productInventoryTransactionCount, + productInventoryTransactionInCurrentLocationCount: productInventoryTransactionInCurrentLocationCount, + productsWithProductInventoryTransactionInCurrentLocation: productsWithProductInventoryTransactionInCurrentLocation?.productCode, productSupplierCount : productSuppliers.size(), ] @@ -296,6 +303,11 @@ class MigrationController { render([count: locations.size(), locations: locations] as JSON) } + def locationsWithProductInventoryTransactions() { + TransactionType transactionType = TransactionType.get(Constants.PRODUCT_INVENTORY_TRANSACTION_TYPE_ID) + def locations = migrationService.getLocationsWithTransaction(transactionType) + render([count: locations.size(), locations: locations] as JSON) + } def productsWithInventoryTransactions() { def location = Location.get(session.warehouse.id) @@ -359,6 +371,20 @@ class MigrationController { } + def migrateProductInventoryTransactions() { + long startTime = System.currentTimeMillis() + Location location = Location.get(session.warehouse.id) + + boolean performMigration = params.boolean("performMigration", false) + Map results = migrationService.migrateProductInventoryTransactions(location, performMigration) + + long responseTime = System.currentTimeMillis() - startTime + String responseTimeMessage = "${performMigration ? 'Migration' : 'Generating preview'} took ${(responseTime)} ms" + log.info "$responseTimeMessage" + + render([responseTime: "$responseTimeMessage", results: results] as JSON) + } + def migrateProductSuppliers(MigrationCommand command) { def startTime = System.currentTimeMillis() diff --git a/grails-app/services/org/pih/warehouse/data/MigrationService.groovy b/grails-app/services/org/pih/warehouse/data/MigrationService.groovy index 784faeaf48e..0a081216271 100644 --- a/grails-app/services/org/pih/warehouse/data/MigrationService.groovy +++ b/grails-app/services/org/pih/warehouse/data/MigrationService.groovy @@ -12,8 +12,11 @@ package org.pih.warehouse.data import grails.gorm.transactions.Transactional import grails.validation.ValidationException import groovy.sql.Sql +import org.hibernate.Criteria import org.hibernate.criterion.CriteriaSpecification import org.hibernate.sql.JoinType +import org.pih.warehouse.DateUtil +import org.pih.warehouse.api.AvailableItem import org.pih.warehouse.core.Constants import org.pih.warehouse.core.Location import org.pih.warehouse.core.LocationType @@ -21,13 +24,14 @@ import org.pih.warehouse.core.LocationTypeCode import org.pih.warehouse.core.Organization import org.pih.warehouse.core.PartyRole import org.pih.warehouse.core.PartyType -import org.pih.warehouse.core.PreferenceTypeCode import org.pih.warehouse.core.RatingTypeCode import org.pih.warehouse.core.RoleType -import org.pih.warehouse.inventory.InventoryItem +import org.pih.warehouse.inventory.ProductAvailabilityService +import org.pih.warehouse.inventory.ProductInventoryTransactionMigrationService import org.pih.warehouse.inventory.Transaction import org.pih.warehouse.inventory.TransactionCode import org.pih.warehouse.inventory.TransactionEntry +import org.pih.warehouse.inventory.TransactionType import org.pih.warehouse.product.Product import org.pih.warehouse.product.ProductSupplier import org.pih.warehouse.receiving.Receipt @@ -41,6 +45,8 @@ class MigrationService { def dataService def gparsService def inventoryService + ProductAvailabilityService productAvailabilityService + ProductInventoryTransactionMigrationService productInventoryTransactionMigrationService def persistenceInterceptor def dataSource @@ -180,11 +186,28 @@ class MigrationService { return results } + def getLocationsWithTransaction(TransactionType transactionType) { + return Transaction.createCriteria().list { + resultTransformer(CriteriaSpecification.ALIAS_TO_ENTITY_MAP) + projections { + inventory { + warehouse { + groupProperty "id", "Location Id" + groupProperty "name", "Location Name" + groupProperty "active", "Location Active" + } + } + count "id", "Transaction Count" + } + eq("transactionType", transactionType) + } + } + def getProductsWithTransactions(Location location, List transactionCodes) { return TransactionEntry.createCriteria().list { projections { inventoryItem { - distinct("product") + property("product", "product") } } transaction { @@ -197,6 +220,23 @@ class MigrationService { order("transactionDate", "asc") order("dateCreated", "asc") } + resultTransformer Criteria.DISTINCT_ROOT_ENTITY + } + } + + List getProductsWithTransactions(Location location, TransactionType transactionType) { + return TransactionEntry.createCriteria().list { + projections { + inventoryItem { + property("product", "product") + } + } + transaction { + eq("transactionType", transactionType) + eq("inventory", location.inventory) + order("transactionDate", "asc") + } + resultTransformer Criteria.DISTINCT_ROOT_ENTITY } } @@ -359,6 +399,128 @@ class MigrationService { return results } + Map migrateProductInventoryTransactions(Location location, boolean performMigration) { + log.info("Migrating Product inventory transactions at location ${location.name} ${performMigration}") + + // 1. Find all transactions with the old Product Inventory type (by PRODUCT_INVENTORY_TRANSACTION_TYPE_ID) + TransactionType oldProductInventoryType = TransactionType.get(Constants.PRODUCT_INVENTORY_TRANSACTION_TYPE_ID) + List transactions = Transaction.findAllByInventoryAndTransactionType(location.inventory, oldProductInventoryType)?.sort { it.transactionDate} + + + Map> results = [:] + recordMigrationResult(results, transactions, true) + + if (performMigration && transactions) { + List products = transactions.transactionEntries?.flatten()?.collect { TransactionEntry te -> te.inventoryItem.product }?.unique() + // Always create a stock snapshot before modifying any transactions in order to prevent quantity on hand + // differences for edge cases that were not addressed + log.debug("Creating inventory baseline for current stock for products found in old transactions") + Transaction currentInventoryBaseline = productInventoryTransactionMigrationService.createInventoryBaselineTransaction( + location, + null, + products, + null, + "Inventory baseline created during old product inventory transactions migration", + null, + true, + true + ) + + transactions.each { Transaction it -> + log.debug "Migrating transaction ${it.transactionNumber} with ${it.transactionEntries.size()} transaction entries" + List entries = it.transactionEntries + // 2. Find all products in entries (and create inventory baseline for these) + List currentTransactionProducts = entries?.collect { TransactionEntry te -> te.inventoryItem.product }?.unique() + + // 3. Create inventory baseline for old transaction that is being migrated (if enabled) + Map availableItems = productAvailabilityService.getAvailableItemsAtDateAsMap( + location, currentTransactionProducts, it.transactionDate) + + Transaction baselineTransaction = productInventoryTransactionMigrationService.createInventoryBaselineTransactionForGivenStock( + location, + null, + availableItems.values(), + it.transactionDate, + "Migrated from single Product Inventory transaction to Baseline + Adjustment pair", + null, + // don't validate transaction date, there is old transaction at the same time, that will be removed + false, + true + ) + if (baselineTransaction) { + baselineTransaction.disableRefresh = true + // Ugly workaround to omit auto-timestamping dateCreated + // Needs to flush it first to be able to overwrite dateCreated + baselineTransaction.save(flush: true) + // Unfortunately, we need to use raw SQL to update the dateCreated field (otherwise it will be + // auto-timestamped) + def sql = new Sql(dataSource) + sql.executeUpdate( + """UPDATE transaction set date_created = ? WHERE id = ?""", + [it.dateCreated, baselineTransaction.id] + ) + + recordMigrationResult(results, [baselineTransaction]) + } + + // 4. Create adjustment transactions for the old transaction that is being migrated + // Date objects are mutable, so we use Instant to clone the date in the command and avoid directly modifying it. + Date adjustmentTransactionDate = DateUtil.asDate(DateUtil.asInstant(it.transactionDate).plusSeconds(1)) + Transaction adjustmentTransaction = createAdjustmentTransaction( + location, + it.transactionEntries, + availableItems, + adjustmentTransactionDate, + "Migrated from single Product Inventory transaction to Baseline + Adjustment pair" + ) + // Ugly workaround to omit auto-timestamping dateCreated + if (adjustmentTransaction?.id) { + Date adjustmentCreatedDate = DateUtil.asDate(DateUtil.asInstant(it.dateCreated).plusSeconds(1)) + // Unfortunately, we need to use raw SQL to update the dateCreated field (otherwise it will be + // auto-timestamped) + def sql = new Sql(dataSource) + sql.executeUpdate( + """UPDATE transaction set date_created = ? WHERE id = ?""", + [adjustmentCreatedDate, adjustmentTransaction.id] + ) + + recordMigrationResult(results, [adjustmentTransaction]) + } + + // 5. Delete the old transaction (with flush, to get rid of that transaction for next calculations) + it.disableRefresh = true + it.delete(flush: true, failOnError: true) + } + + // Zero out stock for products that were migrated but had no stock initially (hence hand no baseline created) + List shouldBeZeroedOut = products - currentInventoryBaseline?.transactionEntries?.collect { it.inventoryItem.product }?.unique() + if (shouldBeZeroedOut) { + log.debug("Check if there is need for 'zeroing out' adjustments for products that had no stock before migration") + + Map availableItems = productAvailabilityService.getAvailableItemsAtDateAsMap( + location, + shouldBeZeroedOut + ) + + createCleanupAdjustment( + location, + availableItems, + "After migration from single Product Inventory transaction to Baseline + Adjustment pair " + + "this transaction is created to ensure that products that had no stock before, " + + "are currently zeroed out too" + ) + } + + // Refresh PA for all products that were migrated + productAvailabilityService.triggerRefreshProductAvailability(location.id, products?.id, true) + } + + return [ + "Location": location?.name, + "Migration Results": results + ] + } + /** * * @param initialQuantity @@ -629,4 +791,125 @@ class MigrationService { return productSuppliers?.size() } + + /** + * Create the adjustment transaction based on the difference between the QoH in the system at the date specified + * by the old product inventory transaction that is being migrated + */ + private Transaction createAdjustmentTransaction(Location facility, + List transactionEntries, + Map availableItems, + Date transactionDate, + String comment) { + // Don't bother populating the transaction's fields until we know we'll need one. + Transaction transaction = new Transaction() + + transactionEntries?.each { TransactionEntry entry -> + // Assuming there is no duplicated product-lot-bin combination in the transaction entries + // If there are, it need to be done similarly like in the Inventory Import + String key = productAvailabilityService.constructAvailableItemKey(entry.binLocation, entry.inventoryItem) + int quantityOnHand = availableItems.get(key)?.quantityOnHand ?: 0 + int adjustmentQuantity = entry.quantity - quantityOnHand + if (adjustmentQuantity == 0) { + return + } + + TransactionEntry transactionEntry = new TransactionEntry( + transaction: transaction, + quantity: adjustmentQuantity, + product: entry.product, + binLocation: entry.binLocation, + inventoryItem: entry.inventoryItem, + comments: entry.comments + ) + transaction.addToTransactionEntries(transactionEntry) + } + + // If there aren't any adjustments, don't bother creating an empty transaction. + if (!transaction.transactionEntries) { + return null + } + + // Now that we know that we need a transaction, we can populate it. + transaction.transactionDate = transactionDate + transaction.transactionType = TransactionType.get(Constants.ADJUSTMENT_CREDIT_TRANSACTION_TYPE_ID) + transaction.transactionNumber = inventoryService.generateTransactionNumber(transaction) + transaction.comment = comment + transaction.inventory = facility.inventory + transaction.disableRefresh = true + + // Needs to be flushed here already, to be able to swap date created on that entry + if (!transaction.save(flush: true, failOnError: true)) { + throw new ValidationException("Invalid transaction", transaction.errors) + } + + return transaction + } + + /** + * Create the adjustment transaction that zeroes out the quantity on hand for products that had no stock before + * the migration process and somehow it's not empty (it might happen due to the backdated transecations) + */ + private Transaction createCleanupAdjustment(Location facility, + Map availableItems, + String comment) { + // Don't bother populating the transaction's fields until we know we'll need one. + Transaction transaction = new Transaction() + + if (availableItems) { + availableItems.each { String key, AvailableItem it -> + int quantityOnHand = it?.quantityOnHand ?: 0 + // If there is already a item with qoh zero, we can skip it + if (quantityOnHand == 0) { + return + } + + TransactionEntry transactionEntry = new TransactionEntry( + transaction: transaction, + quantity: -quantityOnHand, + product: it.inventoryItem.product, + binLocation: it.binLocation, + inventoryItem: it.inventoryItem + ) + transaction.addToTransactionEntries(transactionEntry) + } + } + + // If there aren't any adjustments, don't bother creating an empty transaction. + if (!transaction.transactionEntries) { + return null + } + + // Now that we know that we need a transaction, we can populate it. + transaction.transactionDate = new Date() + transaction.transactionType = TransactionType.get(Constants.ADJUSTMENT_CREDIT_TRANSACTION_TYPE_ID) + transaction.transactionNumber = inventoryService.generateTransactionNumber(transaction) + transaction.comment = comment + transaction.inventory = facility.inventory + transaction.disableRefresh = true + + if (!transaction.save()) { + throw new ValidationException("Invalid transaction", transaction.errors) + } + + return transaction + } + + private static void recordMigrationResult(Map migrationResults, List transactions, Boolean before = false) { + transactions.each {Transaction t -> + t.transactionEntries.each { TransactionEntry te -> + if (!migrationResults.containsKey(te.inventoryItem.product.productCode)) { + migrationResults[te.inventoryItem.product.productCode] = [ + "Status".padRight(10) + "Transaction Number".padRight(25) + "Transaction date".padRight(25) + "CODE".padRight(20) + "ITEMKEY".padRight(60) + "QTY".padRight(10) + ] + } + migrationResults[te.inventoryItem.product.productCode] << "${before ? "BEFORE" : "AFTER"}".padRight(10) + + "${t.transactionNumber.padRight(25)}" + + "${DateUtil.asDateTimeForDisplay(t.transactionDate).padRight(25)}" + + "${t.transactionType.transactionCode.name().padRight(20)}" + + "${te.inventoryItem.product.productCode}:${te.inventoryItem.lotNumber}:${te.binLocation?.name}".padRight(60) + + "${te.quantity.toString().padRight(10)}" + } + } + } } diff --git a/grails-app/services/org/pih/warehouse/inventory/ProductInventoryTransactionMigrationService.groovy b/grails-app/services/org/pih/warehouse/inventory/ProductInventoryTransactionMigrationService.groovy new file mode 100644 index 00000000000..c825153dc39 --- /dev/null +++ b/grails-app/services/org/pih/warehouse/inventory/ProductInventoryTransactionMigrationService.groovy @@ -0,0 +1,25 @@ +package org.pih.warehouse.inventory + +import grails.gorm.transactions.Transactional +import org.pih.warehouse.core.ConfigService +import org.pih.warehouse.importer.ImportDataCommand + +/** + * Responsible for managing product inventory transactions for the old product inventory transaction migration + * to the new Inventory Baseline and Adjustment pair + */ +@Transactional +class ProductInventoryTransactionMigrationService extends ProductInventoryTransactionService { + + ConfigService configService + + @Override + void setSourceObject(Transaction transaction, ImportDataCommand sourceObject) { + // Inventory Import has no source object. + } + + @Override + protected boolean baselineTransactionsEnabled() { + return configService.getProperty("openboxes.transactions.inventoryBaseline.migration.enabled", Boolean) + } +} diff --git a/grails-app/services/org/pih/warehouse/inventory/ProductInventoryTransactionService.groovy b/grails-app/services/org/pih/warehouse/inventory/ProductInventoryTransactionService.groovy index b6e18d45602..e61fc99ffd6 100644 --- a/grails-app/services/org/pih/warehouse/inventory/ProductInventoryTransactionService.groovy +++ b/grails-app/services/org/pih/warehouse/inventory/ProductInventoryTransactionService.groovy @@ -44,6 +44,11 @@ abstract class ProductInventoryTransactionService { * @param transactionDate The datetime that the transaction should be marked with. If left blank will be * the current time. * @param comment An optional comment to associate with the transaction + * @param validateTransactionDates An optional param to disable validation of transactions at the same time + * (Used when for some reason we want to allow multiple transactions at the + * same time). By default it's true. + * @param disableRefresh An optional param to disable the automatic refresh of the product availability when + * the transaction is created. By default it's false. * @return The Transaction that was created */ Transaction createInventoryBaselineTransaction( @@ -52,13 +57,18 @@ abstract class ProductInventoryTransactionService { Collection products, Date transactionDate=null, String comment=null, - Map, String> transactionEntriesComments = [:]) { + Map, String> transactionEntriesComments = [:], + Boolean validateTransactionDates = true, + Boolean disableRefresh = false + ) { List availableItems = productAvailabilityService.getAvailableItemsAtDate( facility, products, transactionDate) createInventoryBaselineTransactionForGivenStock( - facility, sourceObject, availableItems, transactionDate, comment, transactionEntriesComments) + facility, sourceObject, availableItems, transactionDate, comment, transactionEntriesComments, + validateTransactionDates, disableRefresh + ) } /** @@ -80,6 +90,11 @@ abstract class ProductInventoryTransactionService { * @param transactionDate The datetime that the transaction should be marked with. If left blank will be * the current time. * @param comment An optional comment to associate with the transaction + * @param validateTransactionDates An optional param to disable validation of transactions at the same time + * (Used when for some reason we want to allow multiple transactions at the + * same time). By default it's true. + * @param disableRefresh An optional param to disable the automatic refresh of the product availability when + * the transaction is created. By default it's false. * @return The Transaction that was created */ Transaction createInventoryBaselineTransactionForGivenStock( @@ -88,7 +103,10 @@ abstract class ProductInventoryTransactionService { Collection availableItems, Date transactionDate=null, String comment=null, - Map, String> transactionEntriesComments = [:]) { + Map, String> transactionEntriesComments = [:], + validateTransactionDates = true, + disableRefresh = false + ) { // If there are no available items, there would be no transaction entries, so skip creating the transaction. if (!availableItems || !baselineTransactionsEnabled()) { @@ -101,7 +119,7 @@ abstract class ProductInventoryTransactionService { // database level is to the second) so fail if there's already a transaction on the items for the given date. Date actualTransactionDate = transactionDate ?: new Date() List inventoryItems = availableItems.collect{ it.inventoryItem } - if (inventoryService.hasTransactionEntriesOnDate(facility, actualTransactionDate, inventoryItems)) { + if (validateTransactionDates && inventoryService.hasTransactionEntriesOnDate(facility, actualTransactionDate, inventoryItems)) { throw new IllegalArgumentException("A transaction already exists at time ${actualTransactionDate}") } @@ -110,6 +128,7 @@ abstract class ProductInventoryTransactionService { transactionDate: actualTransactionDate, transactionType: transactionType, comment: comment, + disableRefresh: disableRefresh ) transaction.transactionNumber = transactionIdentifierService.generate(transaction) @@ -123,7 +142,7 @@ abstract class ProductInventoryTransactionService { binLocation: availableItem.binLocation, inventoryItem: availableItem.inventoryItem, transaction: transaction, - comments: transactionEntriesComments.get([inventoryItem: availableItem.inventoryItem, binLocation: availableItem.binLocation]), + comments: transactionEntriesComments?.get([inventoryItem: availableItem.inventoryItem, binLocation: availableItem.binLocation]), ) transaction.addToTransactionEntries(transactionEntry) } diff --git a/grails-app/views/migration/dataMigration.gsp b/grails-app/views/migration/dataMigration.gsp index 07d96ea8a13..3f721493b64 100644 --- a/grails-app/views/migration/dataMigration.gsp +++ b/grails-app/views/migration/dataMigration.gsp @@ -57,6 +57,36 @@ + + Product Inventory transactions that should be replaced by Inventory Baseline and Adjustment pair + + ${productInventoryTransactionCount} (total), ${productInventoryTransactionInCurrentLocationCount} (current location) +
+ Products: ${productsWithProductInventoryTransactionInCurrentLocation.join(', ') ?: 'None'} + + +
+ + View All Locations with deprecated Product Inventory transaction + + + Download Inventory (.csv) + + + Preview Migration for Current Location + + + Migrate Current Location + +
+

Warning! Currently it takes about 1-2 minutes to migrate about ~100 transactions. Results will + be visible in the new tab after everything is processed (for your convenience do not close it). + Do not trigger migration for the same location twice (ideally each location should be processed one by one). +
+ Preview displays all transaction entries within this location grouped by product. +

+ + diff --git a/src/main/groovy/org/pih/warehouse/core/Constants.groovy b/src/main/groovy/org/pih/warehouse/core/Constants.groovy index 4cc2dca27be..d01b955544a 100644 --- a/src/main/groovy/org/pih/warehouse/core/Constants.groovy +++ b/src/main/groovy/org/pih/warehouse/core/Constants.groovy @@ -88,6 +88,7 @@ class Constants { // these are direct references to transaction types by primary key static final String CONSUMPTION_TRANSACTION_TYPE_ID = "2" + // Deprecated - This transaction type should not be used to create new transactions static final String ADJUSTMENT_CREDIT_TRANSACTION_TYPE_ID = "3" static final String EXPIRATION_TRANSACTION_TYPE_ID = "4" static final String DAMAGE_TRANSACTION_TYPE_ID = "5" @@ -95,6 +96,7 @@ class Constants { static final String TRANSFER_IN_TRANSACTION_TYPE_ID = "8" static final String TRANSFER_OUT_TRANSACTION_TYPE_ID = "9" static final String ADJUSTMENT_DEBIT_TRANSACTION_TYPE_ID = "10" + // Deprecated - This transaction type should not be used to create new transactions static final String PRODUCT_INVENTORY_TRANSACTION_TYPE_ID = "11" static final String INVENTORY_BASELINE_TRANSACTION_TYPE_ID = "12" From a98acdd2fc463a30436cadd97d56031ab8040c70 Mon Sep 17 00:00:00 2001 From: Walkowiak Date: Thu, 10 Jul 2025 15:20:12 +0200 Subject: [PATCH 2/2] OBPIH-7198 Improvements after review --- grails-app/conf/application.yml | 17 +++++ grails-app/conf/runtime.groovy | 10 --- .../warehouse/data/MigrationController.groovy | 3 +- .../warehouse/data/MigrationService.groovy | 72 +++++++++++-------- ...nventoryTransactionMigrationService.groovy | 2 +- .../org/pih/warehouse/core/Constants.groovy | 2 +- 6 files changed, 65 insertions(+), 41 deletions(-) diff --git a/grails-app/conf/application.yml b/grails-app/conf/application.yml index 1421866b8bb..c7e73a8fc39 100644 --- a/grails-app/conf/application.yml +++ b/grails-app/conf/application.yml @@ -612,6 +612,23 @@ openboxes: enabled: true pageSize: 10 + transactions: + inventoryBaseline: + # OBPIH-7194 + recordStock: + enabled: true + # OBPIH-7195 + inventoryImport: + enabled: true + # OBPIH-7254 + loadDemoData: + enabled: true + # OBPIH-7198 + migration: + enabled: true + # this one is for testing and should be always set to true on production + cleanupAdjustment: true + mail: error: enabled: false diff --git a/grails-app/conf/runtime.groovy b/grails-app/conf/runtime.groovy index 564f72c4189..e78479e4d29 100644 --- a/grails-app/conf/runtime.groovy +++ b/grails-app/conf/runtime.groovy @@ -90,16 +90,6 @@ openboxes.products.merge.enabled = false // Cycle Count configuration (OBPIH-7033) openboxes.cycleCount.products.maxAmount = 50 -// Inventory baseline transaction configuration (OBPIH-7194, OBPIH-7195) -openboxes.transactions.inventoryBaseline.recordStock.enabled = true -openboxes.transactions.inventoryBaseline.inventoryImport.enabled = true - -// Inventory snapshot configuration (OBPIH-7254) -openboxes.transactions.inventoryBaseline.loadDemoData.enabled = true - -// Inventory snapshot configuration (OBPIH-7198) -openboxes.transactions.inventoryBaseline.migration.enabled = true - openboxes.security.rbac.rules = [ [controller: '*', actions: ['delete'], accessRules: [ minimumRequiredRole: RoleType.ROLE_SUPERUSER ]], [controller: '*', actions: ['remove'], accessRules: [ minimumRequiredRole: RoleType.ROLE_SUPERUSER ]], diff --git a/grails-app/controllers/org/pih/warehouse/data/MigrationController.groovy b/grails-app/controllers/org/pih/warehouse/data/MigrationController.groovy index 0f9b9f84c6d..ab921aa328b 100644 --- a/grails-app/controllers/org/pih/warehouse/data/MigrationController.groovy +++ b/grails-app/controllers/org/pih/warehouse/data/MigrationController.groovy @@ -379,7 +379,8 @@ class MigrationController { Map results = migrationService.migrateProductInventoryTransactions(location, performMigration) long responseTime = System.currentTimeMillis() - startTime - String responseTimeMessage = "${performMigration ? 'Migration' : 'Generating preview'} took ${(responseTime)} ms" + String responseTimeMessage = "${performMigration ? 'Migration' : 'Generating preview'} of product inventory " + + "transactions at location ${location.name} took ${(responseTime)} ms" log.info "$responseTimeMessage" render([responseTime: "$responseTimeMessage", results: results] as JSON) diff --git a/grails-app/services/org/pih/warehouse/data/MigrationService.groovy b/grails-app/services/org/pih/warehouse/data/MigrationService.groovy index 0a081216271..dbb27576e9b 100644 --- a/grails-app/services/org/pih/warehouse/data/MigrationService.groovy +++ b/grails-app/services/org/pih/warehouse/data/MigrationService.groovy @@ -9,6 +9,7 @@ **/ package org.pih.warehouse.data +import grails.core.GrailsApplication import grails.gorm.transactions.Transactional import grails.validation.ValidationException import groovy.sql.Sql @@ -49,6 +50,7 @@ class MigrationService { ProductInventoryTransactionMigrationService productInventoryTransactionMigrationService def persistenceInterceptor def dataSource + GrailsApplication grailsApplication def getStockMovementsWithoutShipmentItems() { String query = """ @@ -404,7 +406,7 @@ class MigrationService { // 1. Find all transactions with the old Product Inventory type (by PRODUCT_INVENTORY_TRANSACTION_TYPE_ID) TransactionType oldProductInventoryType = TransactionType.get(Constants.PRODUCT_INVENTORY_TRANSACTION_TYPE_ID) - List transactions = Transaction.findAllByInventoryAndTransactionType(location.inventory, oldProductInventoryType)?.sort { it.transactionDate} + List transactions = Transaction.findAllByInventoryAndTransactionType(location.inventory, oldProductInventoryType)?.sort { it.transactionDate} Map> results = [:] @@ -436,54 +438,46 @@ class MigrationService { Map availableItems = productAvailabilityService.getAvailableItemsAtDateAsMap( location, currentTransactionProducts, it.transactionDate) + String newComment = "Migrated from single Product Inventory transaction to Baseline + Adjustment pair. " + + "Old transaction number: ${it.transactionNumber}, " + + "created by: ${it.createdBy.username}" + "${it.comment ? ', comment: ' + it.comment : ''}" + + // In case old comment is too long, truncate new comment to 255 characters + if (newComment.length() > 255) { + newComment = newComment.substring(0, 255) + } + Transaction baselineTransaction = productInventoryTransactionMigrationService.createInventoryBaselineTransactionForGivenStock( location, null, availableItems.values(), it.transactionDate, - "Migrated from single Product Inventory transaction to Baseline + Adjustment pair", + newComment, null, // don't validate transaction date, there is old transaction at the same time, that will be removed false, true ) if (baselineTransaction) { - baselineTransaction.disableRefresh = true - // Ugly workaround to omit auto-timestamping dateCreated - // Needs to flush it first to be able to overwrite dateCreated - baselineTransaction.save(flush: true) - // Unfortunately, we need to use raw SQL to update the dateCreated field (otherwise it will be - // auto-timestamped) - def sql = new Sql(dataSource) - sql.executeUpdate( - """UPDATE transaction set date_created = ? WHERE id = ?""", - [it.dateCreated, baselineTransaction.id] - ) - + changeDateCreatedOnTransaction(baselineTransaction, it.dateCreated) recordMigrationResult(results, [baselineTransaction]) } // 4. Create adjustment transactions for the old transaction that is being migrated // Date objects are mutable, so we use Instant to clone the date in the command and avoid directly modifying it. + // FIXME Standardize the +1 second for adjustment transaction's transaction date, should be configurable Date adjustmentTransactionDate = DateUtil.asDate(DateUtil.asInstant(it.transactionDate).plusSeconds(1)) Transaction adjustmentTransaction = createAdjustmentTransaction( location, it.transactionEntries, availableItems, adjustmentTransactionDate, - "Migrated from single Product Inventory transaction to Baseline + Adjustment pair" + newComment ) - // Ugly workaround to omit auto-timestamping dateCreated - if (adjustmentTransaction?.id) { - Date adjustmentCreatedDate = DateUtil.asDate(DateUtil.asInstant(it.dateCreated).plusSeconds(1)) - // Unfortunately, we need to use raw SQL to update the dateCreated field (otherwise it will be - // auto-timestamped) - def sql = new Sql(dataSource) - sql.executeUpdate( - """UPDATE transaction set date_created = ? WHERE id = ?""", - [adjustmentCreatedDate, adjustmentTransaction.id] - ) - + if (adjustmentTransaction) { + Date adjustmentDateCreated = DateUtil.asDate(DateUtil.asInstant(it.dateCreated).plusSeconds(1)) + changeDateCreatedOnTransaction(adjustmentTransaction, adjustmentDateCreated) recordMigrationResult(results, [adjustmentTransaction]) } @@ -493,8 +487,9 @@ class MigrationService { } // Zero out stock for products that were migrated but had no stock initially (hence hand no baseline created) + def cleanupAdjustmentEnabled = grailsApplication.config.openboxes.transactions.inventoryBaseline.migration.cleanupAdjustment List shouldBeZeroedOut = products - currentInventoryBaseline?.transactionEntries?.collect { it.inventoryItem.product }?.unique() - if (shouldBeZeroedOut) { + if (cleanupAdjustmentEnabled && shouldBeZeroedOut) { log.debug("Check if there is need for 'zeroing out' adjustments for products that had no stock before migration") Map availableItems = productAvailabilityService.getAvailableItemsAtDateAsMap( @@ -839,7 +834,7 @@ class MigrationService { transaction.disableRefresh = true // Needs to be flushed here already, to be able to swap date created on that entry - if (!transaction.save(flush: true, failOnError: true)) { + if (!transaction.save()) { throw new ValidationException("Invalid transaction", transaction.errors) } @@ -912,4 +907,25 @@ class MigrationService { } } } + + + // FIXME Ugly workaround to omit auto-timestamping dateCreated, we cannot temporarily disable autotimestamping, + // it's available starting at Grails 6 + private void changeDateCreatedOnTransaction(Transaction transaction, Date date = new Date()) { + if (!transaction) { + return + } + + // Needs to flush it first to be able to overwrite dateCreated + transaction.disableRefresh = true + transaction.save(flush: true) + + // Unfortunately, we need to use raw SQL to update the dateCreated field (otherwise it will be + // auto-timestamped) + def sql = new Sql(dataSource) + sql.executeUpdate( + """UPDATE transaction set date_created = ? WHERE id = ?""", + [date, transaction.id] + ) + } } diff --git a/grails-app/services/org/pih/warehouse/inventory/ProductInventoryTransactionMigrationService.groovy b/grails-app/services/org/pih/warehouse/inventory/ProductInventoryTransactionMigrationService.groovy index c825153dc39..defafeb8947 100644 --- a/grails-app/services/org/pih/warehouse/inventory/ProductInventoryTransactionMigrationService.groovy +++ b/grails-app/services/org/pih/warehouse/inventory/ProductInventoryTransactionMigrationService.groovy @@ -15,7 +15,7 @@ class ProductInventoryTransactionMigrationService extends ProductInventoryTransa @Override void setSourceObject(Transaction transaction, ImportDataCommand sourceObject) { - // Inventory Import has no source object. + // Transaction migration has no source object. } @Override diff --git a/src/main/groovy/org/pih/warehouse/core/Constants.groovy b/src/main/groovy/org/pih/warehouse/core/Constants.groovy index d01b955544a..0ee607f2b8f 100644 --- a/src/main/groovy/org/pih/warehouse/core/Constants.groovy +++ b/src/main/groovy/org/pih/warehouse/core/Constants.groovy @@ -88,10 +88,10 @@ class Constants { // these are direct references to transaction types by primary key static final String CONSUMPTION_TRANSACTION_TYPE_ID = "2" - // Deprecated - This transaction type should not be used to create new transactions static final String ADJUSTMENT_CREDIT_TRANSACTION_TYPE_ID = "3" static final String EXPIRATION_TRANSACTION_TYPE_ID = "4" static final String DAMAGE_TRANSACTION_TYPE_ID = "5" + // Deprecated - This transaction type should not be used to create new transactions static final String INVENTORY_TRANSACTION_TYPE_ID = "7" static final String TRANSFER_IN_TRANSACTION_TYPE_ID = "8" static final String TRANSFER_OUT_TRANSACTION_TYPE_ID = "9"