OpenMage / magento-lts

Official OpenMage LTS codebase | Migrate easily from Magento Community Edition in minutes! Download the source code for free or contribute to OpenMage LTS | Security vulnerability patches, bug fixes, performance improvements and more.
https://www.openmage.org
Open Software License 3.0
868 stars 436 forks source link

Widget instance layout updates doesn't store correctly for longer product types #4162

Closed Hanmac closed 3 weeks ago

Hanmac commented 1 month ago

Preconditions (*)

  1. catalog_product_entity.type_id table column is 32 characters
  2. widget_instance_page.page_group table column is 25 characters

Steps to reproduce (*)

  1. Create a new Widget Instance with a Layout Update for a Specific Product Type, The Type has more than 16 characters
  2. Save the Page and Try to reload it.
  3. Because the page_group is created by $typeId . '_products', it can't save the entire name it should have into it.

Expected result (*)

  1. [Screenshots, logs or description]
  2. The Layout Update is correctly restored after save and load

Actual result (*)

  1. [Screenshots, logs or description]
  2. The Layout Update can't be correctly loaded, because the loaded Page Group Name is invalid

See this JS error:

Uncaught TypeError: pageGroup is null
    addPageGroup https://myWebshop/index.php/admin/widget_instance/edit/instance_id/3/type/cms-widget_block/package/default/theme/default/key//back/edit/:1307
    <anonymous> https://myWebshop/index.php/admin/widget_instance/edit/instance_id/3/type/cms-widget_block/package/default/theme/default/key//back/edit/:1533

Possible Fix

widget_instance_page.page_group should have a table column size of: 41 characters = 32 (catalog_product_entity.type_id size) + 9 (_products)

sreichel commented 1 month ago

Can you please provide a sample layout update?

(everything that makes it easier to reproduce helps a lot)

Hanmac commented 1 month ago

@sreichel I mean there isn't much to configure? Just create a Product Type with type name > 16 characters. my Type name was "b3it_virtualevent" with 17 characters.

Screenshot 2024-09-18 at 10-45-25 Neue Widget-Instanz _ Widget-Instanzen _ CMS _ OpenMage Admin

The HTML for this looks like this, notice the ID b3it_virtualevent_products_0

<div class="no-display products group_container" id="b3it_virtualevent_products_0">
   <input type="hidden" class="container_name" name="__[container_name]" value="widget_instance[0][b3it_virtualevent_products]">
   <input type="hidden" name="widget_instance[0][b3it_virtualevent_products][page_id]" value="2">
   <input type="hidden" class="layout_handle_pattern" name="widget_instance[0][b3it_virtualevent_products][layout_handle]" value="default,catalog_product_view,PRODUCT_TYPE_b3it_virtualevent">
   <table cellspacing="0" class="option-header">
      <colgroup>
         <col width="200">
         <col width="220">
         <col width="320">
         <col>
      </colgroup>
      <thead>
         <tr>
            <th>
               <label>Products</label>
            </th>
            <th>
               <label>Block-Referenz 
               <span class="required">*</span>
               </label>
            </th>
            <th>
               <label>Vorlage</label>
            </th>
            <th>&nbsp;</th>
         </tr>
      </thead>
      <tbody>
         <tr>
            <td>
               <input type="radio" class="radio for_all" id="all_b3it_virtualevent_products_0" name="widget_instance[0][b3it_virtualevent_products][for]" value="all" onclick="WidgetInstance.togglePageGroupChooser(this)" checked="checked">&nbsp;
               <label for="all_b3it_virtualevent_products_0">Alle</label>&nbsp;&nbsp;&nbsp;
               <input type="radio" class="radio for_specific" id="specific_b3it_virtualevent_products_0" name="widget_instance[0][b3it_virtualevent_products][for]" value="specific" onclick="WidgetInstance.togglePageGroupChooser(this)">&nbsp;
               <label for="specific_b3it_virtualevent_products_0">Bestimmte Products</label>
            </td>
            <td>
               <div class="block_reference_container">
                  <div class="block_reference"></div>
               </div>
            </td>
            <td>
               <div class="block_template_container">
                  <div class="block_template"></div>
               </div>
            </td>
         </tr>
      </tbody>
   </table>
   <div class="no-display chooser_container" id="b3it_virtualevent_products_ids_0">
      <input type="hidden" class="is_anchor_only" name="widget_instance[0][b3it_virtualevent_products][is_anchor_only]" value="">
      <input type="hidden" class="product_type_id" name="widget_instance[0][b3it_virtualevent_products][product_type_id]" value="b3it_virtualevent">
      <p>
         <input type="text" class="input-text entities" name="widget_instance[0][b3it_virtualevent_products][entities]" value="" readonly="readonly">&nbsp;
         <a class="widget-option-chooser" href="javascript:void(0)" onclick="WidgetInstance.displayEntityChooser('products', 'b3it_virtualevent_products_ids_0')" title="Öffne Auswahlmöglichkeit">
         <img src="/skin/adminhtml/default/default/images/rule_chooser_trigger.gif" class="v-middle" alt="Öffne Auswahlmöglichkeit">
         </a>&nbsp;
         <a href="javascript:void(0)" onclick="WidgetInstance.hideEntityChooser('b3it_virtualevent_products_ids_0')" title="Anwenden">
         <img src="/skin/adminhtml/default/default/images/rule_component_apply.gif" class="v-middle" alt="Anwenden">
         </a>
      </p>
      <div class="chooser"></div>
   </div>
</div>

after save and reload of the page, the Data looks like this:

WidgetInstance.addPageGroup({"page_id":"2","group":"b3it_virtualevent_product","block":"content","for_value":"all","layout_handle":"PRODUCT_TYPE_b3it_virtualevent","b3it_virtualevent_product_entities":"","template":"cms\/widget\/static_block\/default.phtml"});

Notice the group, it is b3it_virtualevent_product instead of b3it_virtualevent_products what it should be?

Because b3it_virtualevent_products doesn't fit the widget_instance_page.page_group table column, so it gets cut to b3it_virtualevent_product

Hanmac commented 1 month ago

This example ProductType is enough to trigger the problem, no other PHP file needed.

Just add this B3It_LongProductName/etc/config.xml

<?xml version="1.0"?>
<!DOCTYPE config>
<config>
    <modules>
        <B3It_LongProductName>
            <version>0.0.1.0</version>
        </B3It_LongProductName>
    </modules>   
    <global>
        <catalog>
            <product>
                <type>
                    <b3it_longproductname translate="label">
                        <label>LongProductName</label>
                        <composite>0</composite>
                        <can_use_qty_decimals>0</can_use_qty_decimals>
                        <is_qty>1</is_qty>
                    </b3it_longproductname>
                    <configurable>
                        <allow_product_types>
                            <b3it_longproductname/>
                        </allow_product_types>
                    </configurable>
                </type>
                <grouped>
                    <allow_product_types>
                        <b3it_longproductname/>
                    </allow_product_types>
                </grouped>
            </product>
        </catalog>
    </global>
</config>

which causes the JS to look like this:

WidgetInstance.addPageGroup({"page_id":"1","group":"b3it_longproductname_prod","block":"content","for_value":"all","layout_handle":"PRODUCT_TYPE_b3it_longproductname","b3it_longproductname_prod_entities":"","template":"cms\/widget\/static_block\/default.phtml"});
sreichel commented 1 month ago

That helps a lot, thanks.

Hanmac commented 1 month ago

I added a possible minimum Size: 41 characters = 32 (catalog_product_entity.type_id column size) + 9 (_products string)

sreichel commented 3 weeks ago

Any suggestions?

Simply change table column seems not to be a good choice.

Hanmac commented 3 weeks ago

Imo I would just simply increase the table column size

Anything else, like not storing "_products" would break existing data

sreichel commented 3 weeks ago

Mhhh ...

1st - you could make your product type shorter 2nd - next one will use a much more longer type-name

How about adding a validation for max length?

Hanmac commented 3 weeks ago

That's why I said the column needs to hold the max possible type name that can be stored in the product table + that '_products' suffix

That the type name in the etc config shouldn't be longer than what the product table can store is another can of worms

sreichel commented 3 weeks ago

Maybe it was a typo to set varchar(25) instead of varchar(255).

Are the any technical reasons to use 25?

sreichel commented 3 weeks ago

Long story short ... PR to increase column length made.