[Closed] FEEDBACK ON BETA of v1.5.5

[DrByte]

Happy New Year ... back at you! Since I've got the chance, I figured that I'd request just a few: more notifiers:

lol

A few questions ...
1. Is there a reason why you want to add NOTIFY_ORDER_CREATE_ADD_PRODUCTS_BEGIN when NOTIFY_ORDER_PROCESSING_STOCK_DECREMENT_INIT already provides that info?

2. What's the purpose of the duplication provided by NOTIFY_ORDER_INVOICE_CONTENT_READY_TO_SEND2?

3 ... more after I review the attributes proposals

[lat9]

lol

A few questions ...
1. Is there a reason why you want to add NOTIFY_ORDER_CREATE_ADD_PRODUCTS_BEGIN when NOTIFY_ORDER_PROCESSING_STOCK_DECREMENT_INIT already provides that info?
Er, because I wasn't paying attention?

2. What's the purpose of the duplication provided by NOTIFY_ORDER_INVOICE_CONTENT_READY_TO_SEND2?
Because NOTIFY_ORDER_INVOICE_CONTENT_READY_TO_SEND doesn't allow the observer to change the contents of the email information blocks.

3 ... more after I review the attributes proposals

See replies embedded above.

[DrByte]

OK, I obviously needed some more coffee before I wrote that. The existing code-block reads:

// 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);
  if ($nick_error) $error = true;
  $zco_notifier->notify('NOTIFY_NICK_CHECK_FOR_DUPLICATE', $nick, $nick_error);
  if ($nick_error) $error = true;

and the highlighted lines need to be removed. Note that the $nick input still needs to be sent on the first notifier!

What about the create_account.php file too?

[DrByte]

Because NOTIFY_ORDER_INVOICE_CONTENT_READY_TO_SEND doesn't allow the observer to change the contents of the email information blocks.

Any objection to us just fixing that?

[DrByte]

Because NOTIFY_ORDER_INVOICE_CONTENT_READY_TO_SEND doesn't allow the observer to change the contents of the email information blocks.

Er, um, in v1.5.5 it does already allow changing them.

[lat9]

What about the create_account.php file too?

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).

[lat9]

Er, um, in v1.5.5 it does already allow changing them.

Where's that coffee ... you're correct, of course. I'll be making some changes in my plugins.

[DrByte]

//-bof-attribute_image_swatch-lat9  *** 3 of 4 ***
                  case ($products_options_names->fields['products_options_type'] == PRODUCTS_OPTIONS_TYPE_SELECT):  // Was default:
//-eof-attribute_image_swatch-lat9  *** 3 of 4 ***
                  // normal dropdown menu display
                  if (isset($_SESSION['cart']->contents[$prod_id]['attributes'][$products_options_names->fields['products_options_id']])) {

                  $options_html_id[] = 'drp-attrib-' . $products_options_names->fields['products_options_id'];
                  $options_menu[] = zen_draw_pull_down_menu('id[' . $products_options_names->fields['products_options_id'] . ']', $products_options_array, $selected_attribute, 'id="' . 'attrib-' . $products_options_names->fields['products_options_id'] . '"') . "\n";
                  $options_comment[] = $products_options_names->fields['products_options_comment'];
                  $options_comment_position[] = ($products_options_names->fields['products_options_comment_position'] == '1' ? '1' : '0');
                  break;
                  
//-bof-attribute_image_swatch-lat9  *** 4 of 4 ***
                  default:
                  $zco_notifier->notify('NOTIFY_ATTRIBUTES_MODULE_DEFAULT_SWITCH', $products_options_names->fields, $options_name, $options_menu, $options_comment, $options_comment_position, $options_html_id);
                  break;
//-eof-attribute_image_swatch-lat9  *** 4 of 4 ***

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...

[lat9]

Any objection to us just fixing that?

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.

[DrByte]

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.