svn commit: r567568 - in /ofbiz/trunk/applications/accounting: script/org/ofbiz/accounting/finaccount/FinAccountServices.xml servicedef/secas.xml servicedef/services_finaccount.xml src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

svn commit: r567568 - in /ofbiz/trunk/applications/accounting: script/org/ofbiz/accounting/finaccount/FinAccountServices.xml servicedef/secas.xml servicedef/services_finaccount.xml src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java

jonesde
Author: jonesde
Date: Mon Aug 20 00:09:47 2007
New Revision: 567568

URL: http://svn.apache.org/viewvc?rev=567568&view=rev
Log:
A number of improvements to FinAccount processing and error messages; most significant one is fixing the updating of the FinAccount balances after a replenishment that was a result of some lazily written code that updated the database using a stale object in memory instead of calling a service to update it, or at least doing a refresh before update, now does the right thing and calls the service

Modified:
    ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/finaccount/FinAccountServices.xml
    ofbiz/trunk/applications/accounting/servicedef/secas.xml
    ofbiz/trunk/applications/accounting/servicedef/services_finaccount.xml
    ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java

Modified: ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/finaccount/FinAccountServices.xml
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/finaccount/FinAccountServices.xml?rev=567568&r1=567567&r2=567568&view=diff
==============================================================================
--- ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/finaccount/FinAccountServices.xml (original)
+++ ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/finaccount/FinAccountServices.xml Mon Aug 20 00:09:47 2007
@@ -178,6 +178,7 @@
         <entity-condition entity-name="FinAccountTrans" list-name="finAccountTransList">
             <condition-expr field-name="finAccountId" env-name="finAccountId"/>
         </entity-condition>
+        <set field="actualBalanceSum" value="0" type="BigDecimal"/>
         <iterate entry-name="finAccountTrans" list-name="finAccountTransList">
             <if>
                 <condition><if-compare field-name="finAccountTrans.finAccountTransTypeId" operator="equals" value="DEPOSIT"/></condition>
@@ -213,8 +214,9 @@
         
         <!-- Okay, now just store the results -->
         <entity-one entity-name="FinAccount" value-name="finAccount"/>
-        <set field="finAccount.actualBalance" from-field="actualBalanceSum"/>
-        <set field="finAccount.availableBalance" from-field="availableBalanceSum"/>
+        <log level="info" message="Updating FinAccount with ID [${finAccountId}] with actualBalance=${actualBalanceSum}, and availableBalance=${availableBalanceSum}"/>
+        <set field="finAccount.actualBalance" from-field="actualBalanceSum" type="Double"/>
+        <set field="finAccount.availableBalance" from-field="availableBalanceSum" type="Double"/>
         <store-value value-name="finAccount"/>
     </simple-method>
 </simple-methods>

Modified: ofbiz/trunk/applications/accounting/servicedef/secas.xml
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/servicedef/secas.xml?rev=567568&r1=567567&r2=567568&view=diff
==============================================================================
--- ofbiz/trunk/applications/accounting/servicedef/secas.xml (original)
+++ ofbiz/trunk/applications/accounting/servicedef/secas.xml Mon Aug 20 00:09:47 2007
@@ -107,7 +107,7 @@
     <!-- financial account transaction ecas -->
     <eca service="finAccountWithdraw" event="return" run-on-error="true">
         <condition field-name="productStoreId" operator="is-not-empty"/>
-        <action service="finAccountReplenish" mode="async" run-as-user="system"/>
+        <action service="finAccountReplenish" mode="async" persist="true" run-as-user="system"/>
     </eca>
 
     <!-- attempt replenish when payment method is updated -->

Modified: ofbiz/trunk/applications/accounting/servicedef/services_finaccount.xml
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/servicedef/services_finaccount.xml?rev=567568&r1=567567&r2=567568&view=diff
==============================================================================
--- ofbiz/trunk/applications/accounting/servicedef/services_finaccount.xml (original)
+++ ofbiz/trunk/applications/accounting/servicedef/services_finaccount.xml Mon Aug 20 00:09:47 2007
@@ -104,6 +104,7 @@
     </service>  
     <service name="expireFinAccountAuth" engine="simple" default-entity-name="FinAccountAuth"
             location="org/ofbiz/accounting/finaccount/FinAccountServices.xml" invoke="expireFinAccountAuth">
+        <!-- NOTE: never set require-new-transaction on this service, needs to be called with other services in same transaction to protect operations such as the payment capture one -->
         <description>Expires a fin account authorization.  Will use current time if no time is supplied in parameter</description>
         <attribute name="finAccountAuthId" type="String" mode="IN" optional="false"/>
         <attribute name="expireDateTime" type="Timestamp" mode="IN" optional="true"/>
@@ -141,6 +142,7 @@
     </service>
     <service name="finAccountWithdraw" engine="java"
             location="org.ofbiz.accounting.finaccount.FinAccountPaymentServices" invoke="finAccountWithdraw" auth="true">
+        <!-- NOTE: never set require-new-transaction on this service, needs to be called with other services in same transaction to protect operations such as the payment capture one -->
         <description>Deposit Funds into a Financial Account</description>
         <attribute name="finAccountId" type="String" mode="IN" optional="false"/>        
         <attribute name="productStoreId" type="String" mode="IN" optional="true"/>

Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java?rev=567568&r1=567567&r2=567568&view=diff
==============================================================================
--- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java (original)
+++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java Mon Aug 20 00:09:47 2007
@@ -148,7 +148,7 @@
                         result.put("authFlag", "0");
                         result.put("authCode", "A");
                         result.put("authRefNum", "0");
-                        Debug.logError("Unable to auth FinAccount: " + result, module);
+                        Debug.logWarning("Unable to auth FinAccount: " + result, module);
                         return result;
                     }
                 }
@@ -163,7 +163,7 @@
                 result.put("authFlag", "0");
                 result.put("authCode", "A");
                 result.put("authRefNum", "0");
-                Debug.logError("Unable to auth FinAccount: " + result, module);
+                Debug.logWarning("Unable to auth FinAccount: " + result, module);
                 return result;
             }
 
@@ -176,13 +176,13 @@
 
                 if (inGoodStanding != null && "N".equals(inGoodStanding)) {
                     Map result = ServiceUtil.returnSuccess();
-                    result.put("authMessage", "Account is currently in bad standing");
+                    result.put("authMessage", "Account is currently not in good standing");
                     result.put("authResult", Boolean.FALSE);
                     result.put("processAmount", amount);
                     result.put("authFlag", "0");
                     result.put("authCode", "A");
                     result.put("authRefNum", "0");
-                    Debug.logError("Unable to auth FinAccount: " + result, module);
+                    Debug.logWarning("Unable to auth FinAccount: " + result, module);
                     return result;
                 }
             }
@@ -202,7 +202,7 @@
                     result.put("authFlag", "0");
                     result.put("authCode", "A");
                     result.put("authRefNum", "0");
-                    Debug.logError("Unable to auth FinAccount: " + result, module);
+                    Debug.logWarning("Unable to auth FinAccount: " + result, module);
                     return result;
                 }
             }
@@ -370,6 +370,21 @@
                 partyId = billToParty.getString("partyId");
             }
         }
+        
+        // BIG NOTE: make sure the expireFinAccountAuth and finAccountWithdraw services are done in the SAME TRANSACTION
+        //(ie no require-new-transaction in either of them AND no running async)
+
+        // cancel the authorization before doing the withdraw to avoid problems with way negative available amount on account; should happen in same transaction to avoid conflict problems
+        Map releaseResult;
+        try {
+            releaseResult = dispatcher.runSync("expireFinAccountAuth", UtilMisc.toMap("userLogin", userLogin, "finAccountAuthId", finAccountAuthId));
+        } catch (GenericServiceException e) {
+            Debug.logError(e, module);
+            return ServiceUtil.returnError(e.getMessage());
+        }
+        if (ServiceUtil.isError(releaseResult)) {
+            return releaseResult;
+        }
 
         // build the withdraw context
         Map withdrawCtx = FastMap.newInstance();
@@ -394,18 +409,6 @@
             return withdrawResp;
         }
 
-        // cancel the authorization
-        Map releaseResult;
-        try {
-            releaseResult = dispatcher.runSync("expireFinAccountAuth", UtilMisc.toMap("userLogin", userLogin, "finAccountAuthId", finAccountAuthId));
-        } catch (GenericServiceException e) {
-            Debug.logError(e, module);
-            return ServiceUtil.returnError(e.getMessage());
-        }
-        if (ServiceUtil.isError(releaseResult)) {
-            return releaseResult;
-        }
-
         // create the capture response
         Map result = ServiceUtil.returnSuccess();
         Boolean processResult = (Boolean) withdrawResp.get("processResult");
@@ -700,27 +703,29 @@
 
         // get the product store settings
         GenericValue finAccountSettings;
+        Map psfasFindMap = UtilMisc.toMap("productStoreId", productStoreId, "finAccountTypeId", finAccount.getString("finAccountTypeId"));
         try {
-            finAccountSettings = delegator.findByPrimaryKeyCache("ProductStoreFinActSetting",
-                    UtilMisc.toMap("productStoreId", productStoreId, "finAccountTypeId",
-                            finAccount.getString("finAccountTypeId")));
+            finAccountSettings = delegator.findByPrimaryKeyCache("ProductStoreFinActSetting", psfasFindMap);
         } catch (GenericEntityException e) {
             Debug.logError(e, module);
             return ServiceUtil.returnError(e.getMessage());
         }
         if (finAccountSettings == null) {
+            Debug.logWarning("finAccountReplenish Warning: not replenishing FinAccount [" + finAccountId  + "] because no ProductStoreFinActSetting record found for: " + psfasFindMap, module);
             // no settings; don't replenish
             return ServiceUtil.returnSuccess();
         }
 
         Double replThres = finAccountSettings.getDouble("replenishThreshold");
         if (replThres == null) {
+            Debug.logWarning("finAccountReplenish Warning: not replenishing FinAccount [" + finAccountId  + "] because ProductStoreFinActSetting.replenishThreshold field was null for: " + psfasFindMap, module);
             return ServiceUtil.returnSuccess();
         }
         BigDecimal replenishThreshold = new BigDecimal(replThres);
 
         BigDecimal replenishLevel = finAccount.getBigDecimal("replenishLevel");
         if (replenishLevel == null || replenishLevel.compareTo(FinAccountHelper.ZERO) == 0) {
+            Debug.logWarning("finAccountReplenish Warning: not replenishing FinAccount [" + finAccountId  + "] because FinAccount.replenishLevel field was null or 0", module);
             // no replenish level set; this account goes not support auto-replenish
             return ServiceUtil.returnSuccess();
         }
@@ -730,6 +735,7 @@
 
         // see if we are within the threshold for replenishment
         if (balance.compareTo(replenishThreshold) > -1) {
+            Debug.logInfo("finAccountReplenish Info: Not replenishing FinAccount [" + finAccountId  + "] because balance [" + balance + "] is greater than the replenishThreshold [" + replenishThreshold + "]", module);
             // not ready
             return ServiceUtil.returnSuccess();        
         }
@@ -750,14 +756,14 @@
         String ownerPartyId = finAccount.getString("ownerPartyId");
         if (ownerPartyId == null) {
             // no owner cannot replenish; (not fatal, just not supported by this account)
-            Debug.logWarning("No owner attached to financial account [" + finAccountId + "] cannot auto-replenish", module);
+            Debug.logWarning("finAccountReplenish Warning: No owner attached to financial account [" + finAccountId + "] cannot auto-replenish", module);
             return ServiceUtil.returnSuccess();
         }
 
         // get the payment method to use to replenish
         String paymentMethodId = finAccount.getString("replenishPaymentId");
         if (paymentMethodId == null) {
-            Debug.logError("No payment method attached to financial account [" + finAccountId + "] cannot auto-replenish", module);
+            Debug.logWarning("finAccountReplenish Warning: No payment method (replenishPaymentId) attached to financial account [" + finAccountId + "] cannot auto-replenish", module);
             return ServiceUtil.returnError("No payment method associated with replenish account");
         }
 
@@ -770,7 +776,7 @@
         }
         if (paymentMethod == null) {
             // no payment methods on file; cannot replenish
-            Debug.logWarning("No payment method found for ID [" + paymentMethodId + "] for party [" + ownerPartyId + "] cannot auto-replenish", module);
+            Debug.logWarning("finAccountReplenish Warning: No payment method found for ID [" + paymentMethodId + "] for party [" + ownerPartyId + "] cannot auto-replenish", module);
             return ServiceUtil.returnError("Cannot locate payment method ID [" + paymentMethodId + "]");
         }
 
@@ -805,22 +811,23 @@
         depositCtx.put("orderItemSeqId", "00001"); // always one item on a replish order
         depositCtx.put("amount",  new Double(depositAmount.doubleValue()));
         depositCtx.put("userLogin", userLogin);
-        Map depositResp;
         try {
-            depositResp = dispatcher.runSync("finAccountDeposit", depositCtx);
+            Map depositResp = dispatcher.runSync("finAccountDeposit", depositCtx);
+            if (ServiceUtil.isError(depositResp)) {
+                return depositResp;
+            }
         } catch (GenericServiceException e) {
             Debug.logError(e, module);
             return ServiceUtil.returnError(e.getMessage());
         }
-        if (ServiceUtil.isError(depositResp)) {
-            return depositResp;
-        }
 
         // say we are in good standing again
-        finAccount.set("inGoodStanding", "Y");
         try {
-            finAccount.store();
-        } catch (GenericEntityException e) {
+            Map ufaResp = dispatcher.runSync("updateFinAccount", UtilMisc.toMap("finAccountId", finAccountId, "inGoodStanding", "Y", "userLogin", userLogin));
+            if (ServiceUtil.isError(ufaResp)) {
+                return ufaResp;
+            }
+        } catch (GenericServiceException e) {
             Debug.logError(e, module);
             return ServiceUtil.returnError(e.getMessage());
         }