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());
>> 
>> 
> 

Reply via email to