Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: Trunk
    • Fix Version/s: 16.11.01
    • Component/s: product
    • Labels:
      None

      Description

      As per the discussion over user list (subject: "Product Name"), user should be able to add product name in product creation process.
      On Catalog >> Products >> New Product section, user won't see any input field to add product name. Same applies to Edit product screen.

      • Add field on the create and edit form.
      • Check for the support in the services used, if not then add support for it.
      1. OFBIZ-7139.patch
        5 kB
        Rishi Solanki
      2. OFBIZ-7139-Updated.patch
        5 kB
        Rishi Solanki

        Issue Links

          Activity

          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Wait, users should be warned that if they set the productName and later want to use content to override the productName it will not work. They will 1st need to remove the productName. See my comment at top of DemoProductI18nData.xml

          Show
          jacques.le.roux Jacques Le Roux added a comment - Wait, users should be warned that if they set the productName and later want to use content to override the productName it will not work. They will 1st need to remove the productName. See my comment at top of DemoProductI18nData.xml
          Hide
          pfm.smits Pierre Smits added a comment -

          No... Good functionality wouldn't require users to remove the product name, when they go the content route. But it could be shown in the ProductContent screen that the product name is overriden by the content variant.

          Show
          pfm.smits Pierre Smits added a comment - No... Good functionality wouldn't require users to remove the product name, when they go the content route. But it could be shown in the ProductContent screen that the product name is overriden by the content variant.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          But it could be shown in the ProductContent screen that the product name is overriden by the content variant.

          That's already done

          Good functionality wouldn't require users to remove the product name, when they go the content route.

          I was bitten by that long ago, did not get an opportunity to look at it again...

          At least it's important to be known in the current context!

          Show
          jacques.le.roux Jacques Le Roux added a comment - But it could be shown in the ProductContent screen that the product name is overriden by the content variant. That's already done Good functionality wouldn't require users to remove the product name, when they go the content route. I was bitten by that long ago, did not get an opportunity to look at it again... At least it's important to be known in the current context!
          Hide
          rishisolankii Rishi Solanki added a comment -

          Thank you Jacques, Pierre for adding more details and inputs on this. It seems to me will need to further look on how system display the product name on UI and how manages the contents. I will try to check further and share details here to discuss further.

          Show
          rishisolankii Rishi Solanki added a comment - Thank you Jacques, Pierre for adding more details and inputs on this. It seems to me will need to further look on how system display the product name on UI and how manages the contents. I will try to check further and share details here to discuss further.
          Hide
          rishisolankii Rishi Solanki added a comment - - edited

          Jacques Le Roux Pierre Smits:

          We have further look into the issue and found that ProductContentWrapper.getProductContentAsText() line 173-205 is responsible for showing productName instead of Product Content of type Product Name. You can see that after getting value from Product.productName it return from the method. Ideally it should look for content first if found then return and if not found then go for Product.productName.

          I would say, add productName on edit product and fix this method will solve all purpose. If no objection then we would provide patch for the same. Content will override the field value in this case.

          Here is code for quick reference;

                  if (productModel.isField(candidateFieldName)) {
                          String candidateValue = product.getString(candidateFieldName);
                          if (UtilValidate.isNotEmpty(candidateValue)) {
                              outWriter.write(candidateValue);
                              return;
                          } else if ("Y".equals(product.getString("isVariant"))) {
                              // look up the virtual product
                              GenericValue parent = ProductWorker.getParentProduct(productId, delegator);
                              if (parent != null) {
                                  candidateValue = parent.getString(candidateFieldName);
                                  if (UtilValidate.isNotEmpty(candidateValue)) {
                                      outWriter.write(candidateValue);
                                      return;
                                  }
                              }
                          }
                  }
          
                  List<GenericValue> productContentList = EntityQuery.use(delegator).from("ProductContent").where("productId", productId, "productContentTypeId", productContentTypeId).orderBy("-fromDate").cache(cache).filterByDate().queryList();
                  if (UtilValidate.isEmpty(productContentList) && ("Y".equals(product.getString("isVariant")))) {
                      GenericValue parent = ProductWorker.getParentProduct(productId, delegator);
                      if (UtilValidate.isNotEmpty(parent)) {
                          productContentList = EntityQuery.use(delegator).from("ProductContent").where("productId", parent.get("productId"), "productContentTypeId", productContentTypeId).orderBy("-fromDate").cache(cache).filterByDate().queryList();
                      }
                  }
                  GenericValue productContent = EntityUtil.getFirst(productContentList);
                  if (productContent != null) {
                      // when rendering the product content, always include the Product and ProductContent records that this comes from
                      Map<String, Object> inContext = new HashMap<String, Object>();
                      inContext.put("product", product);
                      inContext.put("productContent", productContent);
                      ContentWorker.renderContentAsText(dispatcher, delegator, productContent.getString("contentId"), outWriter, inContext, locale, mimeTypeId, partyId, roleTypeId, cache);
                  }
          
          
          Show
          rishisolankii Rishi Solanki added a comment - - edited Jacques Le Roux Pierre Smits : We have further look into the issue and found that ProductContentWrapper.getProductContentAsText() line 173-205 is responsible for showing productName instead of Product Content of type Product Name. You can see that after getting value from Product.productName it return from the method. Ideally it should look for content first if found then return and if not found then go for Product.productName. I would say, add productName on edit product and fix this method will solve all purpose. If no objection then we would provide patch for the same. Content will override the field value in this case. Here is code for quick reference; if (productModel.isField(candidateFieldName)) { String candidateValue = product.getString(candidateFieldName); if (UtilValidate.isNotEmpty(candidateValue)) { outWriter.write(candidateValue); return ; } else if ( "Y" .equals(product.getString( "isVariant" ))) { // look up the virtual product GenericValue parent = ProductWorker.getParentProduct(productId, delegator); if (parent != null ) { candidateValue = parent.getString(candidateFieldName); if (UtilValidate.isNotEmpty(candidateValue)) { outWriter.write(candidateValue); return ; } } } } List<GenericValue> productContentList = EntityQuery.use(delegator).from( "ProductContent" ).where( "productId" , productId, "productContentTypeId" , productContentTypeId).orderBy( "-fromDate" ).cache(cache).filterByDate().queryList(); if (UtilValidate.isEmpty(productContentList) && ( "Y" .equals(product.getString( "isVariant" )))) { GenericValue parent = ProductWorker.getParentProduct(productId, delegator); if (UtilValidate.isNotEmpty(parent)) { productContentList = EntityQuery.use(delegator).from( "ProductContent" ).where( "productId" , parent.get( "productId" ), "productContentTypeId" , productContentTypeId).orderBy( "-fromDate" ).cache(cache).filterByDate().queryList(); } } GenericValue productContent = EntityUtil.getFirst(productContentList); if (productContent != null ) { // when rendering the product content, always include the Product and ProductContent records that this comes from Map< String , Object > inContext = new HashMap< String , Object >(); inContext.put( "product" , product); inContext.put( "productContent" , productContent); ContentWorker.renderContentAsText(dispatcher, delegator, productContent.getString( "contentId" ), outWriter, inContext, locale, mimeTypeId, partyId, roleTypeId, cache); }
          Hide
          julien.nicolas Julien NICOLAS added a comment - - edited

          Ideally it should look for content first if found then return and if not found then go for Product.productName.

          I agree, it seems good like that. It's a easier way if you don't need translation on your product.

          Julien.

          Show
          julien.nicolas Julien NICOLAS added a comment - - edited Ideally it should look for content first if found then return and if not found then go for Product.productName. I agree, it seems good like that. It's a easier way if you don't need translation on your product. Julien.
          Hide
          rishisolankii Rishi Solanki added a comment -

          Here is patch for the same. Thanks to Pratik Kulshreshth for providing fix and verification.

          Show
          rishisolankii Rishi Solanki added a comment - Here is patch for the same. Thanks to Pratik Kulshreshth for providing fix and verification.
          Hide
          rishisolankii Rishi Solanki added a comment -

          Julien NICOLAS thanks for the your input and I'm agree with you on the flow we should opt for. We have provided fix for it, and if community agree then it should be in trunk soon. Thanks!

          Show
          rishisolankii Rishi Solanki added a comment - Julien NICOLAS thanks for the your input and I'm agree with you on the flow we should opt for. We have provided fix for it, and if community agree then it should be in trunk soon. Thanks!
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          I agree on the principle. Note that the same exists for

          1. Categories CategoryContentWrapper.java
          2. Parties PartyContentWrapper.java
          3. ProductConfigItems ProductConfigItemContentWrapper.java
          4. ProductContents ProductContentWrapper.java
          5. ProductPromoContent ProductPromoContentWrapper.java
          6. WorkEffortContent WorkEffortContentWrapper.java

          I though did not check, but for categories, if my comment makes sense in screens...

          Thanks!

          Show
          jacques.le.roux Jacques Le Roux added a comment - I agree on the principle. Note that the same exists for Categories CategoryContentWrapper.java Parties PartyContentWrapper.java ProductConfigItems ProductConfigItemContentWrapper.java ProductContents ProductContentWrapper.java ProductPromoContent ProductPromoContentWrapper.java WorkEffortContent WorkEffortContentWrapper.java I though did not check, but for categories, if my comment makes sense in screens... Thanks!
          Hide
          rishisolankii Rishi Solanki added a comment -

          Jacques Le Roux: Thanks for the pointers, I checked into all the classes having the same problem. Once will have this product name or better to say product fields fix in the system, we will provide fix for all other classes in separate ticket. We have tested the fix and seems it is working fine, please take a look and let us know if it looks fine to you.
          If fix looks good then we will try to provide the same in other files in a day or two.

          Show
          rishisolankii Rishi Solanki added a comment - Jacques Le Roux : Thanks for the pointers, I checked into all the classes having the same problem. Once will have this product name or better to say product fields fix in the system, we will provide fix for all other classes in separate ticket. We have tested the fix and seems it is working fine, please take a look and let us know if it looks fine to you. If fix looks good then we will try to provide the same in other files in a day or two.
          Hide
          rishisolankii Rishi Solanki added a comment - - edited

          We will address remaining classes under - OFBIZ-7169

          Show
          rishisolankii Rishi Solanki added a comment - - edited We will address remaining classes under - OFBIZ-7169
          Hide
          rishisolankii Rishi Solanki added a comment -

          Here is the updated patch, fix the same problem but also prevent system to run unwanted code.

          Show
          rishisolankii Rishi Solanki added a comment - Here is the updated patch, fix the same problem but also prevent system to run unwanted code.
          Hide
          arunpati Arun Patidar added a comment -

          Committed changes in trunk at rev: 1747000.

          Thanks Rishi Solanki for providing patch and your contribution.

          Show
          arunpati Arun Patidar added a comment - Committed changes in trunk at rev: 1747000. Thanks Rishi Solanki for providing patch and your contribution.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          PLease Rishi use short refs for Jira issues, easier to follow status

          Show
          jacques.le.roux Jacques Le Roux added a comment - PLease Rishi use short refs for Jira issues, easier to follow status

            People

            • Assignee:
              arunpati Arun Patidar
              Reporter:
              rishisolankii Rishi Solanki
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development