Never mind my last comment, I see in your message has the other scenarios you are looking for.
Once a user has an address the code to get it from the facility will no longer apply. Is there some way you would like to see this operate differently? -David On Nov 30, 2010, at 4:37 PM, Adam Heath wrote: > 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()); >> >> >
