On 08/26/2010 10:20 PM, jone...@apache.org wrote:
Author: jonesde
Date: Fri Aug 27 03:20:10 2010
New Revision: 990007

URL: http://svn.apache.org/viewvc?rev=990007&view=rev
Log:
Related to 990006 the tax calc address code is made more consistent and 
supported at a lower level by passing in a facilityId when available; this is 
the order component part and the other commit was the lower level accounting 
component part

Modified:
     ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java
     
ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/CheckOutHelper.java

Modified: 
ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java
URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java?rev=990007&r1=990006&r2=990007&view=diff
==============================================================================
--- ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java 
(original)
+++ ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java 
Fri Aug 27 03:20:10 2010
@@ -1529,6 +1529,7 @@ public class OrderServices {

                      // if shippingAddress is still null then don't calculate 
tax; it may be an situation where no tax is applicable, or the data is bad and 
we don't have a way to find an address to check tax for
                      if (shippingAddress == null) {
+                        Debug.logWarning("Not calculating tax for Order [" + orderId + "] because there 
is no shippingAddress, and no address on the origin facility [" +  
orderHeader.getString("originFacilityId") + "]", module);
                          continue;
                      }


Modified: 
ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/CheckOutHelper.java
URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/CheckOutHelper.java?rev=990007&r1=990006&r2=990007&view=diff
==============================================================================
--- 
ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/CheckOutHelper.java
 (original)
+++ 
ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/CheckOutHelper.java
 Fri Aug 27 03:20:10 2010
@@ -56,6 +56,7 @@ import org.ofbiz.order.shoppingcart.prod
  import org.ofbiz.order.shoppingcart.shipping.ShippingEvents;
  import org.ofbiz.order.thirdparty.paypal.ExpressCheckoutEvents;
  import org.ofbiz.party.contact.ContactHelper;
+import org.ofbiz.party.contact.ContactMechWorker;
  import org.ofbiz.product.store.ProductStoreWorker;
  import org.ofbiz.service.GenericServiceException;
  import org.ofbiz.service.LocalDispatcher;
@@ -759,7 +760,7 @@ public class CheckOutHelper {
          int shipGroups = this.cart.getShipGroupSize();
          for (int i = 0; i<  shipGroups; i++) {
              Map<Integer, ShoppingCartItem>  shoppingCartItemIndexMap = new 
HashMap<Integer, ShoppingCartItem>();
-            Map<String, Object>  serviceContext = this.makeTaxContext(i, 
shipAddress, shoppingCartItemIndexMap);
+            Map<String, Object>  serviceContext = this.makeTaxContext(i, 
shipAddress, shoppingCartItemIndexMap, cart.getFacilityId());
              List<List<? extends Object>>  taxReturn = 
this.getTaxAdjustments(dispatcher, "calcTax", serviceContext);

              if (Debug.verboseOn()) Debug.logVerbose("ReturnList: " + 
taxReturn, module);
@@ -786,7 +787,7 @@ public class CheckOutHelper {
          }
      }

-    private Map<String, Object>  makeTaxContext(int shipGroup, GenericValue 
shipAddress, Map<Integer, ShoppingCartItem>  shoppingCartItemIndexMap) {
+    private Map<String, Object>  makeTaxContext(int shipGroup, GenericValue 
shipAddress, Map<Integer, ShoppingCartItem>  shoppingCartItemIndexMap, String 
originFacilityId) {
          ShoppingCart.CartShipInfo csi = cart.getShipInfo(shipGroup);
          int totalItems = csi.shipItemInfo.size();

@@ -835,6 +836,26 @@ public class CheckOutHelper {
              }
          }

+        if (shipAddress == null) {
+            // face-to-face order; use the facility address
+            if (originFacilityId != null) {
+                GenericValue facilityContactMech = 
ContactMechWorker.getFacilityContactMechByPurpose(delegator, originFacilityId, 
UtilMisc.toList("SHIP_ORIG_LOCATION", "PRIMARY_LOCATION"));
+                if (facilityContactMech != null) {
+                    try {
+                        shipAddress = 
delegator.findByPrimaryKey("PostalAddress",
+                                UtilMisc.toMap("contactMechId", 
facilityContactMech.getString("contactMechId")));
+                    } catch (GenericEntityException e) {
+                        Debug.logError(e, module);
+                    }
+                }
+            }
+        }
+
+        // if shippingAddress is still null then don't calculate tax; it may 
be an situation where no tax is applicable, or the data is bad and we don't 
have a way to find an address to check tax for
+        if (shipAddress == null) {
+            Debug.logWarning("Not calculating tax for new order because there is no shipping 
address, no billing address, and no address on the origin facility [" + originFacilityId + 
"]", module);
+        }
+

This is a poor change. Consider the case where cart items are assigned to ship groups, but the actual contactMechId for the group is set much much later.

When a web browser first connects, it is not logged in. The client starts adding products to the cart, and assigning them to shipgroups(based on a nick name). No contactMechId is set at this time. However, there *is* a facilityId, as the store has been configured that way.

So, in this case, this logic will assume that each ship group is a face-to-face sale, which is definately not the case.

So, maybe you would turn off the tax calculation in such a case. That would fix it before the user is logged in.

Now, consider a previous user logging in, and only *some* of their nick-name based shipgroups have pre-existing addresses. Those ship groups will have a contactMechId auto-set, and the checkout process will then prompt for the missing PostalAddress destinations. However, it wants to display correct tax info, so it calculates the taxes. The shipgroups with a contactMechId set will be correct, but then the new groups will again fall back on the facility/face-to-face.

Also, what would happen if all items in a shipgroup were digital, and didn't even need a contactMechId to be set?

          Map<String, Object>  serviceContext = UtilMisc.<String, 
Object>toMap("productStoreId", cart.getProductStoreId());
          serviceContext.put("payToPartyId", cart.getBillFromVendorPartyId());
          serviceContext.put("billToPartyId", cart.getBillToCustomerPartyId());



Reply via email to