Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add NotifierManager::insertContent for easy content insertion. #6064

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
77 changes: 77 additions & 0 deletions includes/classes/traits/NotifierManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,83 @@ public function notify(
}
}

/**
* Requires a define page, and calls regisistered Observers for content output.
*
* @param string $eventID
* @param mixed|array|null $param1 $param1
* @param mixed ...$params
* @return void
*/
function insertContent(string $eventID, $param1 = [], &...$params)
{
global $l;
drbyte marked this conversation as resolved.
Show resolved Hide resolved

// Output any 'define_' page, e.g. CONTENT_FOO will output define_foo.php
$define_page_name = strtolower(str_replace('CONTENT_', 'DEFINE_', $eventID));
$this->outputDefinePage($define_page_name);

$observers = $this->getRegisteredObservers();
if (empty($observers)) {
return;
}
foreach ($observers as $key => $obs) {
// Skip observer if it's not registered for a CONTENT_ event.
if (!str_starts_with($obs['eventID'], 'CONTENT_')) {
continue;
}

// If there is a mapping provided on the observer from event name to define page, use it and continue.
// This allows easy output of define pages with a different name to the CONTENT_* eventID with no extra coding.
// e.g. public array $contentDefinePageMap = [
// 'CONTENT_SOMETHING' => 'define_example_content'
// ];
if (!empty($obs['obs']->contentDefinePageMap) && array_key_exists($eventID, $obs['obs']->contentDefinePageMap)) {
$this->outputDefinePage($obs['obs']->contentDefinePageMap[$eventID]);
continue;
}

// Notify the listening observer that this event has been triggered
$snake_case_method = strtolower($eventID);
if (method_exists($obs['obs'], $snake_case_method)) {
$obs['obs']->{$snake_case_method}($this, $eventID, $params);
} else {
// If no update handler method exists then trigger an error so the problem is logged
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This else clause might be skippable, since it's guarding against older/legacy observer syntax support.
So if what's being built here won't ever need that legacy support, maybe the clause isn't needed.
Not that it's harmful to keep either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this was still useful. If an observer registers interest in CONTENT_FOO and then does not declare a handler function, that's a bug that needs raising. Note that if the contentDefinePageMap contains a mapping, the loop is continued and the error would not be raised, as intended.

Now I think about it, my 'sniff for define page' logic is poor implementation of the design edit removed some rubbish I wrote here :)

FYI I have done a second take of development on a new branch, https://github.com/neekfenwick/zencart/tree/content-notifiers-take-two-issue-6053 as discussed in #6053 (comment) using suggestions from @lat9 to keep the new content notifier separate from NotifierManager, by instantiating a second global $zcc_notifier, similar to $zco_notifier. This work was a little while ago and if it's actually more sensible to keep everything under one hood in NotifierManager I can back out that branch and stick to the original.

$className = (is_object($obs['obs'])) ? get_class($obs['obs']) : $obs['obs'];
trigger_error('WARNING: No update() method (or matching alternative) found in the ' . $className . ' class for event ' . $actualEventId, E_USER_WARNING);
}
}
}

/**
* Write out a named define page, wrapping it in a <div class="define-page"> with extra
* classes and params defined by arguments.
*
* @param string $define_page_name The name of the define page, e.g. define_foo_bar.php
* @param array $classList Any classes to be added to the wrapper e.g. [ 'warning' ]
* @param string $params Any other params to be inserted into the <div> e.g. "id='foobar'"
* @return bool true if output was generated, else false (usually an error)
*/
protected function outputDefinePage (string $define_page_name, array $classList = [], string $params = null): bool {
$define_page = zen_get_define_page_content($define_page_name);
if (empty($define_page)) {
return false;
}
if (empty($classList)) {
$classList = [];
}
if (!empty($params)) {
$params = ' ' . $params;
}
$classList[] = 'define-page';
// Add a class named after the define page for custom styling, e.g. define-contact-us
$classList[] = str_replace('_', '-', $define_page_name);
echo '<div class="' . implode(' ', $classList) . '"' . $params . '>' .
$define_page .
'</div>';
return true;
}

protected function logNotifier($eventID, $param1, $param2, $param3, $param4, $param5, $param6, $param7, $param8, $param9): void
{
if (!defined('NOTIFIER_TRACE') || empty(NOTIFIER_TRACE) || NOTIFIER_TRACE === 'false' || NOTIFIER_TRACE === 'Off') {
Expand Down
35 changes: 35 additions & 0 deletions includes/functions/functions_general.php
Original file line number Diff line number Diff line change
Expand Up @@ -208,3 +208,38 @@ function zen_get_buy_now_button($product_id, string $buy_now_link, $additional_l

return $return_button;
}

/**
* Returns the string content of the named define page, processed either:
* - by an observer running its own processing/substitution/templating engine
* or
* - by the PHP engine (this is the classic default behaviour)
*
* @param string $define_page_name Name of the define page e.g. 'define_contact_us'
* @param array|null $context An object of extra data that may be used by the observer.
* @return string The final content from the define page.
*/
function zen_get_define_page_content(string $define_page_name, ?array $context = null) : string {
global $zco_notifier;

$define_page = zen_get_file_directory(DIR_WS_LANGUAGES . $_SESSION['language'] . '/html_includes/', $define_page_name, false);
if (!file_exists($define_page)) {
trigger_error("Define Page file '$define_page' does not exist!");
return ''; // no action
}

// Give an observer a chance to process the define page
$processed_content = null;
$zco_notifier->notify('NOTIFY_ZEN_PROCESS_DEFINE_PAGE', $define_page, $processed_content, $context);

if (!empty($processed_content)) {
return $processed_content;
}

ob_start();
require($define_page);
$output = ob_get_contents();
ob_end_clean();

return $output;
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@

<?php if ($current_page != FILENAME_CHECKOUT_SUCCESS) { ?>
<h2 id="orderHistoryDetailedOrder"><?php echo HEADING_TITLE . ORDER_HEADING_DIVIDER . sprintf(HEADING_ORDER_NUMBER, zen_output_string_protected($_GET['order_id'])); ?></h2>

<?php }

$zco_notifier->insertContent('CONTENT_ACCOUNT_HISTORY_INFO_INTRO', $order);

$extra_headings = [];
$zco_notifier->notify('NOTIFY_ACCOUNT_HISTORY_INFO_EXTRA_COLUMN_HEADING', $order, $extra_headings);
?>

<table id="orderHistoryHeading">
<tr class="tableHeading">
<th scope="col" id="myAccountQuantity"><?php echo HEADING_QUANTITY; ?></th>
Expand Down Expand Up @@ -105,6 +105,10 @@

</div>

<?php
$zco_notifier->insertContent('CONTENT_ACCOUNT_HISTORY_INFO_POST_ORDER', $order);
?>

<?php
/**
* Used to display any downloads associated with the cutomers account
Expand Down Expand Up @@ -184,4 +188,7 @@
<div><?php echo $order->info['payment_method']; ?></div>
</div>
<br class="clearBoth">
<?php
$zco_notifier->insertContent('CONTENT_ACCOUNT_HISTORY_INFO_EXTRO', $order);
?>
</div>