Account History Info order product list, new notifiers so additional data columns can be provided by Observer

[neekfenwick]

account_history_info screen shows a table of products in the order with a fixed table structure. The admin screen orders.php has a couple of notifier hooks that allow you to add to the columns of data with an observer, see

zencart/admin/orders.php

Line 860 in 24ac52e

$zco_notifier->notify('NOTIFY_ADMIN_ORDERS_CONTENT_UNDER_PRODUCTS', ['oID' => $oID], $extra_content);

and

zencart/admin/orders.php

Line 946 in 24ac52e

$zco_notifier->notify('NOTIFY_ADMIN_ORDERS_STATUS_HISTORY_EXTRA_COLUMN_DATA', $orders_history->fields, $extra_data);

I am adding columns to our account_history_info screen so our customers can see the latest status of each item in our production system. I may as well make this flexible and contribute it.

I'm using exactly the same structure as in the orders.php screen, so the two new notifiers are:

NOTIFY_ACCOUNT_HISTORY_INFO_EXTRA_COLUMN_HEADING

Columns are described in an array like:

[
    [ 'align' => 'center', 'text' => 'Heading goes here' ]
]

NOTIFY_ACCOUNT_HISTORY_INFO_EXTRA_COLUMN_DATA

[
    [ 'align' => 'center', 'text' => 'Data goes here' ]
]

[neekfenwick]

PR #6051 created, hasn't linked to this Issue for some reason.

Question: what's the opinion on adding a define page to the account history info page, so admin can introduce messages onto that page? In this case, we want messaging relating to the extra columns of data.

For example, the index page has some rather overblown logic:

filenames.php:
define('FILENAME_DEFINE_MAIN_PAGE', 'define_main_page');

index/header_php.php:
$define_page = zen_get_file_directory(DIR_WS_LANGUAGES . $_SESSION['language'] . '/html_includes/', FILENAME_DEFINE_MAIN_PAGE, 'false');

includes/templates/template_default/templates/tpl_index_default.php:
<div id="indexDefaultMainContent" class="content"><?php require($define_page); ?></div>

Rather than get involved with a new constant, we could use the $current_page (set up in init_sanitize.php), so in tpl_account_history_info.php:

<?php
$define_page = zen_get_file_directory(DIR_WS_LANGUAGES . $_SESSION['language'] . '/html_includes/', "define_{$current_page}", 'false');
if (file_exists($define_page)) {
?>
<div id="<?php echo $current_page ?>MainContent" class="content">
  <?php require($define_page); ?>
</div>
<?php } ?>

Which results in bringing in html_includes/define_account_history_info.php in a block like this:

<div id="account_history_infoMainContent">
  <p>Account history info define here.</p>
</div>

I'm not sure what to do with the id of the div, and if it should have a class. In my mind, pretty much every specific element should have a class, both specific to itself and to its general type of thing, e.g. class="define-account-history-info define-page". Of course if you need page specific rules, the page itself has an id="accountHistInfo" element wrapping it, so one could use a CSS rule like #accountHistInfo .define-page to target define page content. As you can see, I'm not very familiar with the zen cart style guide 😀

[lat9]

1. The reason that the PR wasn't linked to this issue is that the Fixes #6050 needs to be in the body of the comment for the PR, not in its title.
2. Unfortunately, there's no (to my knowledge) Zen Cart style guide. What you've suggested above (a) sounds like another good consolidating idea, (b) should be a separate issue/PR and (c) should be a function (or maybe 2, one to format the $define_page and another to do the remaining processing).

[lat9]

For the notifier processing suggested, I'll note that the admin side can be more 'rigid' since there's no templating system. That's why using a returned array of [ 'align' => 'center', 'text' => 'Data goes here' ] works, because it's known that the current (fixed) layout in the admin defines a class named text-center.

For the storefront, the notification (and subsequent processing) has no clue as to what (if any) alignment classes are defined in the current template's CSS layout. I'd suggest, for the storefront, using a params entry instead of align to let the observer specify any HTML parameters to be associated with any <th> or <td> data supplied.

[neekfenwick]

For the storefront, the notification (and subsequent processing) has no clue as to what (if any) alignment classes are defined in the current template's CSS layout. I'd suggest, for the storefront, using a params entry instead of align to let the observer specify any HTML parameters to be associated with any <th> or <td> data supplied.

Good point. I've jiggled the PR to have this change instead. I'm not sure how this helps completely, as an addon contributing columns isn't going to have any knowledge of the current template's CSS either, but I can see my code copied from admin was pretty naive.

I wonder what the thinking is to have concrete Classes for some of these data structures and not the loose associative arrays we all know and love? It's such a pain to have to instantiate and initialise class instances, but it does bring with it the joys of strong typing and documented members.

[neekfenwick]

1. The reason that the PR wasn't linked to this issue is that the Fixes #6050 needs to be in the body of the comment for the PR, not in its title.

Thanks, I never seem to get this right :)

2. Unfortunately, there's no (to my knowledge) Zen Cart style guide. What you've suggested above (a) sounds like another good consolidating idea, (b) should be a separate issue/PR and (c) should be a function (or maybe 2, one to format the $define_page and another to do the remaining processing).

I'd suggest a system a little like the Notifiers, so the base code can say "insert whatever content is defined for THIS_POINT_IN_CODE here", for example like we have:

$zco_notifier->notify('NOTIFY_ACCOUNT_HISTORY_INFO_EXTRA_COLUMN_DATA', [ 'order' => $order, 'orders_product' => $op ], $extra_data);

We could have

$zco_notifier->insertContent(DEFINE_ACCOUNT_HISTORY_INFO_INTRO, $order);

and I presume we could have either a define page like html_includes/define_account_history_info_intro.php files alongside all the other define_xyz.php files, or a similar hooks system to the Notifier where it calls a handler function provided by addons etc that have registered an interest in providing content for that point in the page. It's super late here, more thought tomorrow, and as you say this belongs in a separate Issue/PR.

[lat9]

For the storefront, the notification (and subsequent processing) has no clue as to what (if any) alignment classes are defined in the current template's CSS layout. I'd suggest, for the storefront, using a params entry instead of align to let the observer specify any HTML parameters to be associated with any <th> or <td> data supplied.

Good point. I've jiggled the PR to have this change instead. I'm not sure how this helps completely, as an addon contributing columns isn't going to have any knowledge of the current template's CSS either, but I can see my code copied from admin was pretty naive.

I wonder what the thinking is to have concrete Classes for some of these data structures and not the loose associative arrays we all know and love? It's such a pain to have to instantiate and initialise class instances, but it does bring with it the joys of strong typing and documented members.

At a minimum, having these notifications in the template-rendering would make it easier when/if your base template is updated so that you don't have to merge your changes into the base template's updated files. I agree that the notification addition will not be as useful to a general-use plugin, although having a set of 'params' would allow that general-use plugin to provide its own CSS to ensure 'proper' styling.

[neekfenwick]

2. Unfortunately, there's no (to my knowledge) Zen Cart style guide. What you've suggested above (a) sounds like another good consolidating idea, (b) should be a separate issue/PR and (c) should be a function (or maybe 2, one to format the $define_page and another to do the remaining processing).

I've opened Issue #6053 with an idea for extending the Notifier system with a Define Page output mechanism.