-
Notifications
You must be signed in to change notification settings - Fork 55
Add isKlaviyoEnabled to product/viewed.phtml #337
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
Conversation
Logic in product/viewed.phtml is always executed even when you disable the module.
Add isKlaviyoEnabled check to product/viewed.phtml
smoucka
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @GerritJanHypoStore . Can you please update CHANGELOG.md as well?
|
@smoucka I've updated the CHANGELOG.md and version numbers. Please let me know if there's anything else you'd like me to adjust. |
|
@GerritJanHypoStore mind rollin back the version bump and updating the changelog to put this fix under Unreleased? We will likely release it alongside some other changes so we dont want to bump the version just yet |
|
@cykolln I’ve rolled back the version bump, updated the changelog to place the fix under Unreleased, and merged it with the latest version. Please let me know if anything else needs to be updated. |
|
@GJHypostore Sorry for the delay here and thank you again. Can you please rebase? We'll get this merged in once that's done. |
|
@smoucka No worries at all, I’ve gone ahead and rebased as requested. |
Logic in product/viewed.phtml is always executed even when you disable the module.
|
@smoucka I noticed the changelog issue flagged by the automated tests, so I went ahead and redid the rebase to fix it. |
| /** @var Klaviyo\Reclaim\Block\Catalog\Product\ViewedProduct $block */ | ||
| if (!$block->isKlaviyoEnabled()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Klaviyo: Missing API Key Validation Causes Render Issues
The template only checks isKlaviyoEnabled() but doesn't verify that a public API key exists. The block's _toHtml() method checks both conditions: isKlaviyoEnabled() && getPublicApiKey(). Other templates like analytics/initialize.phtml also check both. If Klaviyo is enabled but no API key is configured, the template will render and attempt to call getViewedProductJson() and getViewedItemJson(), which may cause issues. The check should be: if (!($block->isKlaviyoEnabled() && $block->getPublicApiKey())) to match the block's logic.
Description
Add isKlaviyoEnabled to product/viewed.phtml. The logic in this phtml file gets executed even if you disable the module. Other phtml files do have an check.
Manual Testing Steps
Verified that product/viewed.phtml executes regardless of whether the Klaviyo module is enabled or disabled.
Added a check for isKlaviyoEnabled at the top of product/viewed.phtml, matching the logic used in other phtml files.
Confirmed that the Klaviyo script no longer renders when the module is disabled.
Confirmed that the script still renders correctly when the module is enabled.