Hello Scott, Please see my comments inline:
-- Ashish On Wed, Sep 30, 2009 at 4:39 PM, Scott Gray <[email protected]>wrote: > Inline > > > On 30/09/2009, at 11:57 PM, Ashish Vijaywargiya wrote: > > 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? >> > > Personally I think it is most common that people would want variants > excluded from the search results and only have the virtual product listed I totally agree with you on this. > > > 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? >> > > Yes I think it fine to keep the demo data set to "Y", even though it has > the same effect as leaving it out, the option becomes more explicit to > anyone looking at the data. Changes are reverted in r820249. > > > >> >>> >>> 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"/> >>>> >>>> >>>> >>>> >>> >
