-
Notifications
You must be signed in to change notification settings - Fork 35
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
fix google tag rendering #3978
fix google tag rendering #3978
Conversation
Front-end summary Node 18
|
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.
LGTM now
helpers/Layout.php
Outdated
$gaTag = $_ENV['GA_TAG']; | ||
$environment = $_ENV['NODE_ENV'] === 'production' ? 'Production' : 'Internal'; |
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.
We need to check if the keys are set in $_ENV now before accessing them to avoid undefined-key warnings.
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.
actually $_ENV['GA_TAG']
will be null if it not define as far as I can see
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.
Yes. But it will also raise a PHP warning :)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3978 +/- ##
==========================================
Coverage 27.31% 27.31%
Complexity 11021 11021
==========================================
Files 927 927
Lines 33965 33965
==========================================
Hits 9276 9276
Misses 24689 24689 ☔ View full report in Codecov by Sentry. |
Version
There are 0 BREAKING CHANGE, 0 feature, 1 fix |
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.
Well done, @poyuki!
Let's merge and port to the March release:
- On top of https://github.com/oat-sa/tao-core/releases/tag/v54.2.2
- Then as part of TAO Community
- Then include TAO Community to https://github.com/oat-sa/taocloud-ami-luoss/blob/release-2024.03.2/composer.json#L103
Oh, and after that, April release will have to be updated too of course. I guess we can just set the latest tao-core version there.
TR-6036
Summary
This PR aim to address issue with rendering of GA tag in some environments
Development impact
<? ?>
(which at least onuat.eu.premium.lab.taocloud.org
converted into html comment)$_ENV
instead ofgetenv
(usage of which discouraged by dotenv lib documentation )