Hello Scott,

Thanks for your comment.
Please see my comments inline:

On Wed, Sep 30, 2009 at 3:37 PM, Scott Gray <[email protected]>wrote:

> Hi Ashish
>
> Comment inline
>
> Regards
> Scott
>
> On 30/09/2009, at 9:03 PM, [email protected] wrote:
>
>  Author: ashish
>> Date: Wed Sep 30 08:03:27 2009
>> New Revision: 820204
>>
>> URL: http://svn.apache.org/viewvc?rev=820204&view=rev
>> Log:
>> Putting the default value for prodSearchExcludeVariants=Y in ProductStore
>> entity. (This is what at present happening OOTB)
>> Also I had put a not-empty check so if someone forgot to put this value in
>> demo data then by default it will include variant products.
>>
>> Modified:
>>
>>  
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductSearchSession.java
>>  ofbiz/trunk/specialpurpose/ecommerce/data/DemoProduct.xml
>>
>> Modified:
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductSearchSession.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductSearchSession.java?rev=820204&r1=820203&r2=820204&view=diff
>>
>> ==============================================================================
>> ---
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductSearchSession.java
>> (original)
>> +++
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductSearchSession.java
>> Wed Sep 30 08:03:27 2009
>> @@ -767,7 +767,7 @@
>>       }
>>
>>       // check the ProductStore to see if we should add the
>> ExcludeVariantsConstraint
>> -        if (productStore != null &&
>> !"N".equals(productStore.getString("prodSearchExcludeVariants"))) {
>> +        if (productStore != null &&
>> UtilValidate.isNotEmpty(productStore.getString("prodSearchExcludeVariants"))
>> && !"N".equals(productStore.getString("prodSearchExcludeVariants"))) {
>>           searchAddConstraint(new
>> ProductSearch.ExcludeVariantsConstraint(), session);
>>           // not consider this a change for now, shouldn't change often:
>> constraintsChanged = true;
>>       }
>>
>>
> Here you're change null == Y to null == N
> I don't think it is a good idea to change the default behavior unless we
> have a good reason to do so, this would affect anyone taking an update who
> hasn't set this value in the past.


Do you think that its good to exclude variant if no value is provided? I
agree that after taking update someone will get affected by my change. Do
you think it is ok if I revert my changes of Java code and keep the default
value as "Y" in demo data for prodSearchExcludeVariants?


>
>
>  Modified: ofbiz/trunk/specialpurpose/ecommerce/data/DemoProduct.xml
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/ecommerce/data/DemoProduct.xml?rev=820204&r1=820203&r2=820204&view=diff
>>
>> ==============================================================================
>> --- ofbiz/trunk/specialpurpose/ecommerce/data/DemoProduct.xml (original)
>> +++ ofbiz/trunk/specialpurpose/ecommerce/data/DemoProduct.xml Wed Sep 30
>> 08:03:27 2009
>> @@ -57,7 +57,7 @@
>>       authFraudMessage="Your order has been rejected and your account has
>> been disabled due to fraud."
>>       authErrorMessage="Problem connecting to payment processor; we will
>> continue to retry and notify you by email."
>>       storeCreditValidDays="90" storeCreditAccountEnumId="FIN_ACCOUNT"
>> -        visualThemeId="EC_DEFAULT" autoApproveInvoice="Y"
>> shipIfCaptureFails="Y" autoApproveOrder="Y" showOutOfStockProducts="Y"/>
>> +        visualThemeId="EC_DEFAULT" prodSearchExcludeVariants="Y"
>> autoApproveInvoice="Y" shipIfCaptureFails="Y" autoApproveOrder="Y"
>> showOutOfStockProducts="Y"/>
>>
>>   <!-- <ProductStorePaymentSetting productStoreId="9000"
>> paymentMethodTypeId="CREDIT_CARD" paymentServiceTypeEnumId="PRDS_PAY_AUTH"
>> paymentService="testRandomAuthorize"/> -->
>>   <ProductStorePaymentSetting productStoreId="9000"
>> paymentMethodTypeId="CREDIT_CARD" paymentServiceTypeEnumId="PRDS_PAY_AUTH"
>> paymentService="alwaysApproveCCProcessor"
>> paymentCustomMethodId="CC_AUTH_ALWAYSAPPROV"/>
>>
>>
>>
>

Reply via email to