[Closed] FEEDBACK ON BETA of v1.5.5

[DrByte]

Nope, in the create-account process you want to ensure that the nickname chosen is unique; in the account-edit process, the nickname's already chosen (and not modifiable).

Okay. I'm staging these changes here: https://github.com/zencart/zencart/pull/733

[lat9]

Can you talk a bit about the sort of testing you've done, and logic considered, to ensure there are no ill side-effects of changing this default?
I know Ajeh's probably grinding fingernails on the blackboard trying to anticipate potential problems with this...

Those notifiers are (as the comments imply) used by one of my commercial plugins. That plugin adds a brand-new option type and the majority of the notifiers in the attributes.php file allow the plugin to deal with this.

The one thing that I did run into when changing the default processing was the "configuration_group 0" mis-handling (i.e. when configuration_group_id=0 gets wiped from the configuration table by improperly-coded SQL install scripts). The change in Zen Cart v1.5.4 (I think) where those configuration items are now moved to configuration group 6 (modules) corrected that issue (which is also included as part of that plugin's distribution).

[dw08gm]

Submit buttons readiness for cssButtons

If this has not already been done, can we have class="button" OR class="submitButton" AND/OR (perhaps) id="submit_Button" OR id="submit_Button" uniformly inserted within all <input type="submit"...> for cssButton readiness.

includes/modules/pages/checkout_success/header_php.php // class="submitButton" and id="submitbutton"
includes/modules/pages/payer_auth_auth/header_php.php
includes/modules/pages/payer_auth_verifier/header_php.php
includes/modules/payment/authorizenet/authorizenet_admin_notification.php
includes/templates/template_default/sideboxes/tpl_search.php
includes/templates/template_default/sideboxes/tpl_search_header.php

admin/banner_statistics.php
admin/categories.php
admin/coupon_restrict.php
admin/developers_tool_kit.php
admin/document_general.php
admin/document_product.php
admin/includes/modules/newsletters/product_notification.php
admin/login.php // class="button" and id="btn_submit"
admin/media_manager.php
admin/option_name.php
admin/option_values.php
admin/password_forgotten.php // class="button"
admin/product.php
admin/product_free_shipping.php
admin/product_music.php
admin/products_to_categories.php

Thanks in advance.

[mc12345678]

Bulletin-board notifier support. Needs a change to /includes/modules/pages/account_edit/header_php.php to be able to distinguish between an email-address check for the current customer vs. an overall check for the email address, changing:

// check external hook for duplicate email address, so we can reject the change if duplicates aren't allowed externally
  // (the observers should set any messageStack output as needed)
  $nick_error = false;
  $zco_notifier->notify('NOTIFY_NICK_CHECK_FOR_EXISTING_EMAIL', $email_address, $nick_error);

to

// check external hook for duplicate email address, so we can reject the change if duplicates aren't allowed externally
  // (the observers should set any messageStack output as needed)
  $nick_error = false;
  $zco_notifier->notify('NOTIFY_NICK_CHECK_FOR_EXISTING_EMAIL', $email_address, $nick_error, $nick);

Otherwise, a customer with a bulletin-board account can't change their password because the (unchanged) email address already associated with the nickname is going to come back as "already used".

But as a header_php.php file, doesn't it actually have direct access to $nick by declaring the variable as global within the observer making the passing of the value(s) unnecessary? This way the observer knows the value of $nick and any change will be available to follow after the notify was executed.

I mean I guess yeah the value could be passed to the observer, but it seems excessive unless there is some plan to modify the observer class operation to restrict it's access to only information specifically fed to it. Though the extra "hint" about what data should be handled does help those that seek to design further...

[mc12345678]

The reason that I'd created another notifier to add those fields (that it turns out isn't needed!) is that I didn't want to introduce version-specific notifiers! The last thing that I, as a plugin developer, want is to have to know which version of Zen Cart is being used for my plugin to determine how to handle a specific notifier.

Yes, I fully appreciate that challenge. But it's a necessary evil. We do try to defer "big" changes to "big" versions, but I'm a believer in progressive change especially since some people delay upgrading terribly long but try to use new plugins against old code so having new hooks or improvements integrated sooner than later tends to be better. Fortunately there are a few ways to handle version control, including just testing whether a certain thing has been passed or not, etc.

I try to do a mix of both, though I could imagine with a sufficient quantity of plugins and enough versions of ZC it will/can get to a point of too much.. Really can make a simple action take up a lot of code space covering for pre 1.5.3 versions where the observer class is limited to just the array of immutable data passed by the notifier and there is a need to change something that is to follow... (Meaning, very thankful for the added features of the observer class at least providing the 9 additional modifiable variables, though haven't considered packaging that up to use on older ZC versions.. Mostly for some of what is described above, want updating to continue to occur for safety/security.)

[mc12345678]

Can you talk a bit about the sort of testing you've done, and logic considered, to ensure there are no ill side-effects of changing this default?
I know Ajeh's probably grinding fingernails on the blackboard trying to anticipate potential problems with this...

I had been considering adding these notifiers into an attributes.php modifying file, but also was somewhat wondering about the rework of the case... Why not at the current default insert a begin and end notifier allowing the observer code to either take action or let the default code do its part. That way don't run into the issue that lat9 identified regarding the value of 0 problem

case default:
$zco_notifier->notify('NOTIFY_ATTRIBUTES_MODULE_DEFAULT_SWITCH_BEGIN', $products_options_names->fields, $options_name, $options_menu, $options_comment, $options_comment_position, $options_html_id);
default code to occur...
$zco_notifier->notify('NOTIFY_ATTRIBUTES_MODULE_DEFAULT_SWITCH_END', $products_options_names->fields, $options_name, $options_menu, $options_comment, $options_comment_position, $options_html_id);

[mc12345678]

While we're on the notifier request path:

includes/templates/templates_default/common/tpl_footer.php

at the end add:

$zco_notifier->notify('NOTIFY_FOOTER_END');

or add some form of notifier into includes/templates/templates_default/common/tpl_main_page.php that would activate on each and every page load preferably towards the start of the page?

Plugin User Tracking adds the above notifier, but of course being in the footer, if anything prevents the footer from being processed (intentionally or not) then it doesn't get activated. Headers have notifiers, but newly created pages would be missed as the specific notify must be identified and if not mistaken it is not possible to use some form of pattern match to which to attach, therefore would have to initially identify all existing header notifiers, then users would have to 1) validate that they update the particular header notifier when they copy/create a new page and 2) add the particular notifier to the list of headers to which to observe...

[mc12345678]

includes/modules/pages/checkout_success/header_php.php

suggest modifying line 99 (99-104 shown below):

  $products_query = "SELECT products_id, products_name
                     FROM " . TABLE_ORDERS_PRODUCTS . "
                     WHERE orders_id = :ordersID
                     ORDER BY products_name";
  $products_query = $db->bindVars($products_query, ':ordersID', $orders_id, 'integer');
  $products = $db->Execute($products_query);

  $products_query = "SELECT DISTINCT products_id, products_name
                     FROM " . TABLE_ORDERS_PRODUCTS . "
                     WHERE orders_id = :ordersID
                     ORDER BY products_name";
  $products_query = $db->bindVars($products_query, ':ordersID', $orders_id, 'integer');
  $products = $db->Execute($products_query);

Prevents repetitive display of the product name in an order that has the same products_id multiple times.

[mc12345678]

admin/options_name_manager.php

      case 'delete_option':
        // demo active test
        if (zen_admin_demo()) {
          $_GET['action']= '';
          $messageStack->add_session(ERROR_ADMIN_DEMO, 'caution');
          zen_redirect(zen_href_link(FILENAME_OPTIONS_NAME_MANAGER, $_SESSION['page_info'] . '&option_order_by=' . $option_order_by));
        }
        $option_id = zen_db_prepare_input($_GET['option_id']);

        $remove_option_values = $db->Execute("select products_options_id, products_options_values_id from " . TABLE_PRODUCTS_OPTIONS_VALUES_TO_PRODUCTS_OPTIONS . " where products_options_id='" . (int)$option_id . "'");

        while (!$remove_option_values->EOF) {
         $zco_notifier->notify ('OPTIONS_NAME_MANAGER_DELETE_OPTION', array ('option_id' => $option_id, 'options_values_id' => (int)$remove_option_values->fields['products_options_values_id'] ));
          $db->Execute("delete from " . TABLE_PRODUCTS_OPTIONS_VALUES . " where products_options_values_id='" . (int)$remove_option_values->fields['products_options_values_id'] . "' and products_options_values_id !=0");
          $remove_option_values->MoveNext();
        }

Under update_options_values

        } else {
          // action delete
          while (!$all_update_products->EOF) {
            // get all option_values
            $all_options_values = $db->Execute("select products_options_id, products_options_values_id from " . TABLE_PRODUCTS_OPTIONS_VALUES_TO_PRODUCTS_OPTIONS . " where products_options_id='" . (int)$_POST['options_id'] . "'");
            $updated = 'false';
           while (!$all_options_values->EOF) {
              $check_all_options_values = $db->Execute("select products_attributes_id from " . TABLE_PRODUCTS_ATTRIBUTES . " where products_id='" . (int)$all_update_products->fields['products_id'] . "' and options_id='" . (int)$all_options_values->fields['products_options_id'] . "' and options_values_id='" . (int)$all_options_values->fields['products_options_values_id'] . "'");
              if ($check_all_options_values->RecordCount() >= 1) {
                // delete for this product with Option Name options_value_id
// echo '<br>This should be deleted: ' . zen_get_products_name($all_options_values->fields['products_options_id']);
// change to delete
// should add download delete
                $db->Execute("delete from " . TABLE_PRODUCTS_ATTRIBUTES . " where products_id='" . (int)$all_update_products->fields['products_id'] . "' and options_id='" . (int)$_POST['options_id'] . "'");
                $zco_notifier->notify('OPTIONS_NAME_MANAGER_UPDATE_OPTIONS_VALUES_DELETE', array('products_id'=>$all_update_products->fields['products_id'], 'options_id'=>$all_options_values->fields['products_options_id'], 'options_values_id'=>$all_options_values->fields['products_options_values_id']));
              } else {

Totally open to the naming of the above and combination of data to be provided. Anything that would provide at least that amount of information would better than nothing.

[lat9]

Thank you for the notifier updates that I'd requested in post #205. There were, unfortunately, two that didn't (yet?) make it to /includes/modules/attributes.php:

               $tmp_attributes_image = '';
                $tmp_attributes_image_row = 0;
                $show_attributes_qty_prices_icon = 'false';
                
//-bof-attribute_image_swatch-lat9  *** 1 of 4 ***
                $zco_notifier->notify('NOTIFY_ATTRIBUTES_MODULE_START_OPTION', $products_options_names->fields);
//-eof-attribute_image_swatch-lat9  *** 1 of 4 ***

                while (!$products_options->EOF) {
                  // reset
                  $products_options_display_price='';
                  $new_attributes_price= '';
                  $price_onetime = '';

                  $products_options_array[] = array('id' => $products_options->fields['products_options_values_id'],
                  'text' => $products_options->fields['products_options_values_name']);

//-bof-posm_price_weight-lat9  *** 1 of 1 ***
                  $zco_notifier->notify ('NOTIFY_ATTRIBUTES_MODULE_START_OPTIONS_LOOP', array (), $products_options->fields);
//-eof-posm_price_weight-lat9  *** 1 of 1 ***