Cross Sell Advanced [Support Thread]

[DivaVocals]

NOTE: you really need to read through that file to find any other similar issues ...

Well if I run the query

SELECT configuration_group_id FROM zen_configuration_group WHERE configuration_group_title = 'Deluxe Cross Sell' LIMIT 1;

I get zero results. So wouldn't the delete statement be deleting nothing and NOT the 0 configuration group???

[grantopt]

Well if I run the query

SELECT configuration_group_id FROM zen_configuration_group WHERE configuration_group_title = 'Deluxe Cross Sell' LIMIT 1;

I get zero results. So wouldn't the delete statement be deleting nothing and NOT the 0 configuration group???

I see that this is the same thing going on when I asked in the Testimonials manager about those inserts in the uninstall sql. And Ajeh responded that those are 3 stock configs for ZC. After research under certain circumstances it can remove the 3 stock configs. So the 3 inserts plus another for IH was there as a safety when the sql deletes the id for 0.

After reading, there has to be a "and xxx != 0" as a fallback in case the mod id equates to 0.

[Ajeh]

I would suggest posting your full proposed fix, before contributing it so that it can be checked ...

[DivaVocals]

Not the SAME thing at all.. The Testimonials Manager SQL is just plain old WRONG because it inadvertently sets a configuration group id variable to configuration group '0'. (read my response in that thread -- I DO state the issue and solution)

The SQL in question for this module DOES NOT do this..

I see that this is the same thing going on when I asked in the Testimonials manager about those inserts in the uninstall sql. And Ajeh responded that those are 3 stock configs for ZC. After research under certain circumstances it can remove the 3 stock configs. So the 3 inserts plus another for IH was there as a safety when the sql deletes the id for 0.

After reading, there has to be a "and xxx != 0" as a fallback in case the mod id equates to 0.

No fall back required.. simply DO NOT set a variable for configuration group id '0'.. Just don't write SQL scripts which delete ANYTHING from configuration_group '0'

[DivaVocals]

I would suggest posting your full proposed fix, before contributing it so that it can be checked ...

Are you responding to my previous post.. because HONESTLY I am still not seeing the issue with the SQL.. If I query for a configuration group that does not exist, I can't possibly delete anything from it let alone configuration group ID '0'.. I WANT to fix it if it's broken, but I'm REALLY not understanding why it's wrong..

**confused**

[lat9]

As I tiptoe into the discussion, I'd like to propose the following changes to the code fragment that Ajeh posted in post #9. The issue, as I see it, is that if the 'old title' didn't exist, the $result object wouldn't have any fields and, with mySQL's conversion in effect the value '$xsell_old_configuration_id' would be magically converted to (you guessed it) 0.

/* Find configuation group ID of Previous Version of Cross Sell */
        $sql = "SELECT configuration_group_id FROM ".TABLE_CONFIGURATION_GROUP." WHERE configuration_group_title='".$xsell_old_menu_title."' LIMIT 1";
        $result = $db->Execute($sql);
        $xsell_old_configuration_id = $result->fields['configuration_group_id'];

        /* Remove Previous Version of Cross Sell from the configuration group table */
        $sql = "DELETE FROM ".TABLE_CONFIGURATION_GROUP." WHERE configuration_group_id ='".$xsell_old_configuration_id."'";
        $db->Execute($sql);

        /* Remove Previous Version of Cross Sell items from the configuration table */
        $sql = "DELETE FROM ".TABLE_CONFIGURATION." WHERE configuration_group_id ='".$xsell_old_configuration_id."'";
        $db->Execute($sql);

I'd suggest a small recoding of the fragment as follows:

/* Find configuation group ID of Previous Version of Cross Sell */
        $sql = "SELECT configuration_group_id FROM ".TABLE_CONFIGURATION_GROUP." WHERE configuration_group_title='".$xsell_old_menu_title."' LIMIT 1";
        $result = $db->Execute($sql);
        // -----
        // Only remove the old configuration if it exists!
        //
        if (!$result->EOF) {
          $xsell_old_configuration_id = $result->fields['configuration_group_id'];

           /* Remove Previous Version of Cross Sell from the configuration group table */
          $sql = "DELETE FROM ".TABLE_CONFIGURATION_GROUP." WHERE configuration_group_id ='".$xsell_old_configuration_id."'";
          $db->Execute($sql);

          /* Remove Previous Version of Cross Sell items from the configuration table */
          $sql = "DELETE FROM ".TABLE_CONFIGURATION." WHERE configuration_group_id ='".$xsell_old_configuration_id."'";
          $db->Execute($sql);
        }

[Ajeh]

As a test, temporarily create the file:
/your_secret_admin/test_delete.php

with the code:

<?php
require('includes/application_top.php');


    $xsell_old_menu_title = 'Cross Sell';
    $xsell_menu_title = 'Cross Sell Advanced';
    $xsell_menu_text = 'Cross Sell Advanced Configuration';

echo 'Previous Version of Cross Sell: ' . '<br><br>';
      /* Find configuation group ID of Previous Version of Cross Sell */
      $sql = "SELECT configuration_group_id FROM ".TABLE_CONFIGURATION_GROUP." WHERE configuration_group_title='".$xsell_old_menu_title."' LIMIT 1";
echo 'XSELL CODE what would be deleted TABLE_CONFIGURATION_GROUP <br>$sql: ' . $sql . '<br><br>';
      $result = $db->Execute($sql);
        $xsell_old_configuration_id = $result->fields['configuration_group_id'];
echo '$xsell_old_configuration_id set to: ' . $xsell_old_configuration_id . '<br><br>';

      /* Remove Previous Version of Cross Sell from the configuration group table */
      $sql = "DELETE FROM ".TABLE_CONFIGURATION_GROUP." WHERE configuration_group_id ='".$xsell_old_configuration_id."'";
//        $db->Execute($sql);
echo 'XSELL CODE what would be deleted TABLE_CONFIGURATION_GROUP <br>$sql: ' . $sql . '<br><br>';


// test results with SELECT
echo '==================================' . '<br><br>';
      $sql = "SELECT * FROM ".TABLE_CONFIGURATION_GROUP." WHERE configuration_group_id ='".$xsell_old_configuration_id."'";
      $chk_sql =  $db->Execute($sql);
echo 'TEST XSELL CODE what SELECT finds in TABLE_CONFIGURATION_GROUP <br>$sql: ' . $sql . '<br><br>';
echo '<strong>RecordCount found TABLE_CONFIGURATION_GROUP: ' . $chk_sql->RecordCount() . '</strong><br>';

// if found what would be deleted
while (!$chk_sql->EOF) {
  echo 'configuration_group_id: ' . $chk_sql->fields['configuration_group_id'] . '<br><br>';
  $chk_sql->MoveNext();
}

// test results with SELECT
      /* Remove Previous Version of Cross Sell items from the configuration table */
      $sql = "DELETE FROM ".TABLE_CONFIGURATION." WHERE configuration_group_id ='".$xsell_old_configuration_id."'";
echo '==================================' . '<br><br>';
echo 'TEST XSELL CODE what would be deleted TABLE_CONFIGURATION <br>$sql: ' . $sql . '<br><br>';
echo '$xsell_old_configuration_id set to: ' . $xsell_old_configuration_id . '<br><br>';
echo 'XSELL CODE what DELETE would be run on TABLE_CONFIGURATION <br>$sql: ' . $sql . '<br><br>';
//        $db->Execute($sql);
      $sql = "SELECT * FROM ".TABLE_CONFIGURATION." WHERE configuration_group_id ='".$xsell_old_configuration_id."'";
      $chk_sql =  $db->Execute($sql);

echo 'XSELL CODE what SELECT finds in TABLE_CONFIGURATION <br>$sql: ' . $sql . '<br><br>';
echo '<strong>RecordCount found TABLE_CONFIGURATION: ' . $chk_sql->RecordCount() . '</strong><br><br>';

echo 'What would be removed: ' . '<br>';
// if found what would be deleted
while (!$chk_sql->EOF) {
  echo 'configuration_key: ' . $chk_sql->fields['configuration_key'] . '<br>';
  $chk_sql->MoveNext();
}
echo '==================================' . '<br><br>';

While kind of a scrappy file, it would show you the results of what gets deleted if there isn't a value for: $xsell_old_configuration_id

and what would end up deleted from the configuration table ...

This will not delete anything as it uses SELECTS rather than actual DELETE on the tables but does show you the content of the $sql and what keys would get wiped out ...

Lat9's fix would add a safety on this portion of the code, and it needs to be done similarly on anything else doing DELETE in this file ...

[DivaVocals]

As a test, temporarily create the file:
/your_secret_admin/test_delete.php

with the code:

<?php
require('includes/application_top.php');


    $xsell_old_menu_title = 'Cross Sell';
    $xsell_menu_title = 'Cross Sell Advanced';
    $xsell_menu_text = 'Cross Sell Advanced Configuration';

echo 'Previous Version of Cross Sell: ' . '<br><br>';
      /* Find configuation group ID of Previous Version of Cross Sell */
      $sql = "SELECT configuration_group_id FROM ".TABLE_CONFIGURATION_GROUP." WHERE configuration_group_title='".$xsell_old_menu_title."' LIMIT 1";
echo 'XSELL CODE what would be deleted TABLE_CONFIGURATION_GROUP <br>$sql: ' . $sql . '<br><br>';
      $result = $db->Execute($sql);
        $xsell_old_configuration_id = $result->fields['configuration_group_id'];
echo '$xsell_old_configuration_id set to: ' . $xsell_old_configuration_id . '<br><br>';

      /* Remove Previous Version of Cross Sell from the configuration group table */
      $sql = "DELETE FROM ".TABLE_CONFIGURATION_GROUP." WHERE configuration_group_id ='".$xsell_old_configuration_id."'";
//        $db->Execute($sql);
echo 'XSELL CODE what would be deleted TABLE_CONFIGURATION_GROUP <br>$sql: ' . $sql . '<br><br>';


// test results with SELECT
echo '==================================' . '<br><br>';
      $sql = "SELECT * FROM ".TABLE_CONFIGURATION_GROUP." WHERE configuration_group_id ='".$xsell_old_configuration_id."'";
      $chk_sql =  $db->Execute($sql);
echo 'TEST XSELL CODE what SELECT finds in TABLE_CONFIGURATION_GROUP <br>$sql: ' . $sql . '<br><br>';
echo '<strong>RecordCount found TABLE_CONFIGURATION_GROUP: ' . $chk_sql->RecordCount() . '</strong><br>';

// if found what would be deleted
while (!$chk_sql->EOF) {
  echo 'configuration_group_id: ' . $chk_sql->fields['configuration_group_id'] . '<br><br>';
  $chk_sql->MoveNext();
}

// test results with SELECT
      /* Remove Previous Version of Cross Sell items from the configuration table */
      $sql = "DELETE FROM ".TABLE_CONFIGURATION." WHERE configuration_group_id ='".$xsell_old_configuration_id."'";
echo '==================================' . '<br><br>';
echo 'TEST XSELL CODE what would be deleted TABLE_CONFIGURATION <br>$sql: ' . $sql . '<br><br>';
echo '$xsell_old_configuration_id set to: ' . $xsell_old_configuration_id . '<br><br>';
echo 'XSELL CODE what DELETE would be run on TABLE_CONFIGURATION <br>$sql: ' . $sql . '<br><br>';
//        $db->Execute($sql);
      $sql = "SELECT * FROM ".TABLE_CONFIGURATION." WHERE configuration_group_id ='".$xsell_old_configuration_id."'";
      $chk_sql =  $db->Execute($sql);

echo 'XSELL CODE what SELECT finds in TABLE_CONFIGURATION <br>$sql: ' . $sql . '<br><br>';
echo '<strong>RecordCount found TABLE_CONFIGURATION: ' . $chk_sql->RecordCount() . '</strong><br><br>';

echo 'What would be removed: ' . '<br>';
// if found what would be deleted
while (!$chk_sql->EOF) {
  echo 'configuration_key: ' . $chk_sql->fields['configuration_key'] . '<br>';
  $chk_sql->MoveNext();
}
echo '==================================' . '<br><br>';

While kind of a scrappy file, it would show you the results of what gets deleted if there isn't a value for: $xsell_old_configuration_id

and what would end up deleted from the configuration table ...

This will not delete anything as it uses SELECTS rather than actual DELETE on the tables but does show you the content of the $sql and what keys would get wiped out ...

Lat9's fix would add a safety on this portion of the code, and it needs to be done similarly on anything else doing DELETE in this file ...

At last.. thanks ladies.. It's not that I didn't believe there WAS a problem.. I just wasn't quite sure how it was an issue (thanks for the explanation lat9!! I GET IT now.. ) I will make the change and re-submit when I return from New York next week..

GIRL POWER ROCKS!!!

[Ajeh]

Again, you might post your customizations for this file before you submit it to the add-ons to ensure the fixes are complete ...

[DivaVocals]

Again, you might post your customizations for this file before you submit it to the add-ons to ensure the fixes are complete ...

yes ma'am will do!