svn commit: r1025874 - in /ofbiz/trunk: applications/accounting/src/org/ofbiz/accounting/tax/TaxAuthorityServices.java framework/common/src/org/ofbiz/common/geo/GeoWorker.java

[jonesde]

Author: jonesde
Date: Thu Oct 21 08:05:19 2010
New Revision: 1025874

URL: http://svn.apache.org/viewvc?rev=1025874&view=rev
Log:
Changed rateProductTaxCalc service to expand Geos differently, using a Map so there is only one geoId for each geoTypeId and when the IDs are expanded by regions it prioritizes current values over new ones, especially values from the original PostalAddress have priority over expanded regions like a countyGeoId on the PostalAddress will be used instead of one (or more) expanded based on the zip code

Modified:
    ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/tax/TaxAuthorityServices.java
    ofbiz/trunk/framework/common/src/org/ofbiz/common/geo/GeoWorker.java

Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/tax/TaxAuthorityServices.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/tax/TaxAuthorityServices.java?rev=1025874&r1=1025873&r2=1025874&view=diff
==============================================================================
--- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/tax/TaxAuthorityServices.java (original)
+++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/tax/TaxAuthorityServices.java Thu Oct 21 08:05:19 2010
@@ -27,6 +27,7 @@ import java.util.Map;
 import java.util.Set;
 
 import javolution.util.FastList;
+import javolution.util.FastMap;
 import javolution.util.FastSet;
 
 import org.ofbiz.base.util.Debug;
@@ -181,7 +182,12 @@ public class TaxAuthorityServices {
             }
         }
         if (shippingAddress == null || (shippingAddress.get("countryGeoId") == null && shippingAddress.get("stateProvinceGeoId") == null && shippingAddress.get("postalCodeGeoId") == null)) {
-            return ServiceUtil.returnError("The address(es) used for tax calculation did not have State/Province or Country or other tax jurisdiction values set, so we cannot determine the taxes to charge.");
+            String errMsg = "The address(es) used for tax calculation did not have State/Province or Country or other tax jurisdiction values set, so we cannot determine the taxes to charge.";
+            if (shippingAddress != null) {
+                errMsg += " [ID:" + shippingAddress.getString("contactMechId") + ", Address 1: " + shippingAddress.get("address1") + ", Postal Code/ID: " + shippingAddress.get("postalCodeGeoId") + "/" + shippingAddress.get("postalCodeGeoId") + ", State/Province: " + shippingAddress.get("stateProvinceGeoId") + ", Country: " + shippingAddress.get("countryGeoId") + "]";
+                Debug.logError(errMsg, module);
+            }
+            return ServiceUtil.returnError(errMsg);
         }
 
         // without knowing the TaxAuthority parties, just find all TaxAuthories for the set of IDs...
@@ -228,31 +234,31 @@ public class TaxAuthorityServices {
     }
 
     private static void getTaxAuthorities(Delegator delegator, GenericValue shippingAddress, Set taxAuthoritySet) throws GenericEntityException {
-        Set geoIdSet = FastSet.newInstance();
+        Map geoIdByTypeMap = FastMap.newInstance();
         if (shippingAddress != null) {
             if (UtilValidate.isNotEmpty(shippingAddress.getString("countryGeoId"))) {
-                geoIdSet.add(shippingAddress.getString("countryGeoId"));
+                geoIdByTypeMap.put("COUNTRY", shippingAddress.getString("countryGeoId"));
             }
             if (UtilValidate.isNotEmpty(shippingAddress.getString("stateProvinceGeoId"))) {
-                geoIdSet.add(shippingAddress.getString("stateProvinceGeoId"));
+                geoIdByTypeMap.put("STATE", shippingAddress.getString("stateProvinceGeoId"));
             }
             if (UtilValidate.isNotEmpty(shippingAddress.getString("countyGeoId"))) {
-                geoIdSet.add(shippingAddress.getString("countyGeoId"));
+                geoIdByTypeMap.put("COUNTY", shippingAddress.getString("countyGeoId"));
             }
             String postalCodeGeoId = ContactMechWorker.getPostalAddressPostalCodeGeoId(shippingAddress, delegator);
             if (UtilValidate.isNotEmpty(postalCodeGeoId)) {
-                geoIdSet.add(postalCodeGeoId);
+                geoIdByTypeMap.put("POSTAL_CODE", postalCodeGeoId);
             }
         } else {
             Debug.logWarning("shippingAddress was null, adding nothing to taxAuthoritySet", module);
         }
 
-        //Debug.logInfo("Tax calc geoIdSet before expand:" + geoIdSet + "; this is for shippingAddress=" + shippingAddress, module);
+        //Debug.logInfo("Tax calc geoIdByTypeMap before expand:" + geoIdByTypeMap + "; this is for shippingAddress=" + shippingAddress, module);
         // get the most granular, or all available, geoIds and then find parents by GeoAssoc with geoAssocTypeId="REGIONS" and geoIdTo=<granular geoId> and find the GeoAssoc.geoId
-        geoIdSet = GeoWorker.expandGeoRegionDeep(geoIdSet, delegator);
-        //Debug.logInfo("Tax calc geoIdSet after expand:" + geoIdSet, module);
+        geoIdByTypeMap = GeoWorker.expandGeoRegionDeep(geoIdByTypeMap, delegator);
+        //Debug.logInfo("Tax calc geoIdByTypeMap after expand:" + geoIdByTypeMap, module);
 
-        List taxAuthorityRawList = delegator.findList("TaxAuthority", EntityCondition.makeCondition("taxAuthGeoId", EntityOperator.IN, geoIdSet), null, null, null, true);
+        List taxAuthorityRawList = delegator.findList("TaxAuthority", EntityCondition.makeCondition("taxAuthGeoId", EntityOperator.IN, geoIdByTypeMap.values()), null, null, null, true);
         taxAuthoritySet.addAll(taxAuthorityRawList);
         //Debug.logInfo("Tax calc taxAuthoritySet after expand:" + taxAuthoritySet, module);
     }

Modified: ofbiz/trunk/framework/common/src/org/ofbiz/common/geo/GeoWorker.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/src/org/ofbiz/common/geo/GeoWorker.java?rev=1025874&r1=1025873&r2=1025874&view=diff
==============================================================================
--- ofbiz/trunk/framework/common/src/org/ofbiz/common/geo/GeoWorker.java (original)
+++ ofbiz/trunk/framework/common/src/org/ofbiz/common/geo/GeoWorker.java Thu Oct 21 08:05:19 2010
@@ -19,9 +19,11 @@
 package org.ofbiz.common.geo;
 
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import javolution.util.FastList;
+import javolution.util.FastMap;
 import javolution.util.FastSet;
 
 import org.ofbiz.base.util.Debug;
@@ -85,22 +87,24 @@ public class GeoWorker {
         return geoList;
     }
 
-    public static Set<String> expandGeoRegionDeep(Set<String> geoIdSet, Delegator delegator) throws GenericEntityException {
-        if (UtilValidate.isEmpty(geoIdSet)) {
-            return geoIdSet;
-        }
-        Set<String> geoIdSetTemp = FastSet.newInstance();
-        for (String curGeoId: geoIdSet) {
-            List<GenericValue> geoAssocList = delegator.findByAndCache("GeoAssoc", UtilMisc.toMap("geoIdTo", curGeoId, "geoAssocTypeId", "REGIONS"));
+    public static Map<String, String> expandGeoRegionDeep(Map<String, String> geoIdByTypeMapOrig, Delegator delegator) throws GenericEntityException {
+        if (UtilValidate.isEmpty(geoIdByTypeMapOrig)) {
+            return geoIdByTypeMapOrig;
+        }
+        Map<String, String> geoIdByTypeMapTemp = FastMap.newInstance();
+        for (Map.Entry<String, String> geoIdByTypeEntry: geoIdByTypeMapOrig.entrySet()) {
+            List<GenericValue> geoAssocList = delegator.findByAndCache("GeoAssoc", UtilMisc.toMap("geoIdTo", geoIdByTypeEntry.getValue(), "geoAssocTypeId", "REGIONS"));
             for (GenericValue geoAssoc: geoAssocList) {
-                geoIdSetTemp.add(geoAssoc.getString("geoId"));
+                GenericValue newGeo = delegator.findOne("Geo", true, "geoId", geoAssoc.getString("geoId"));
+                geoIdByTypeMapTemp.put(newGeo.getString("geoTypeId"), newGeo.getString("geoId"));
             }
         }
-        geoIdSetTemp = expandGeoRegionDeep(geoIdSetTemp, delegator);
-        Set<String> geoIdSetNew = FastSet.newInstance();
-        geoIdSetNew.addAll(geoIdSet);
-        geoIdSetNew.addAll(geoIdSetTemp);
-        return geoIdSetNew;
+        geoIdByTypeMapTemp = expandGeoRegionDeep(geoIdByTypeMapTemp, delegator);
+        Map<String, String> geoIdByTypeMapNew = FastMap.newInstance();
+        // add the temp Map first, then the original over top of it, ie give the original priority over the sub/expanded values
+        geoIdByTypeMapNew.putAll(geoIdByTypeMapTemp);
+        geoIdByTypeMapNew.putAll(geoIdByTypeMapOrig);
+        return geoIdByTypeMapNew;
     }
 
     public static boolean containsGeo(List<GenericValue> geoList, String geoId, Delegator delegator) {