-
Notifications
You must be signed in to change notification settings - Fork 1
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
WordPress側の情報をmetaタグに出力する #56
Conversation
WalkthroughThis update primarily focuses on deprecating certain features and introducing new ones in the Kunoichi GaCommunicator plugin. It removes dependencies, adds deprecation notices, introduces a new "Meta tags" section in the WordPress settings page, and modifies the behavior of some functions. The changes aim to enhance user experience and maintain up-to-date practices. Changes
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
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.
Files selected (8)
- assets/js/ga-communicator-setting.js (1)
- assets/js/ga-custom-dimensions.js (1)
- src/Kunoichi/GaCommunicator/Api/Accounts.php (1)
- src/Kunoichi/GaCommunicator/Api/Profiles.php (1)
- src/Kunoichi/GaCommunicator/Api/Properties.php (1)
- src/Kunoichi/GaCommunicator/Screen/Settings.php (8)
- src/Kunoichi/GaCommunicator/Utility/PlaceHolders.php (1)
- src/Kunoichi/GaCommunicator/Utility/ScriptRenderer.php (2)
Files ignored due to filter (2)
- dist/css/ga-communicator-setting.css
- dist/css/map/ga-communicator-setting.css.map
Files skipped from review due to trivial changes (2)
- assets/js/ga-communicator-setting.js
- assets/js/ga-custom-dimensions.js
Review comments generated (14)
- Review: 7
- LGTM: 7
Tips
Chat with
CodeRabbit Bot (@coderabbitai
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitai
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignore
anywhere in the PR description to pause further reviews from the bot.
@@ -7,6 +7,7 @@ | |||
* Profiles list. | |||
* | |||
* @package ga-communicator | |||
* @deprecated 3.5.0 |
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.
The class Profiles
has been marked as deprecated. Ensure that all references to this class in the codebase are updated or removed to prevent future issues. If this class is part of a public API, make sure to communicate this change properly to the users and provide them with an alternative if possible.
- * @deprecated 3.5.0
+ * @deprecated 3.5.0 Use the new replacement class instead.
@@ -6,6 +6,7 @@ | |||
* Accounts api. | |||
* | |||
* @package ga-communicator | |||
* @deprecated 3.5.0 |
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.
The class Properties
is marked as deprecated but there's no alternative suggested. It's a good practice to suggest an alternative when deprecating a class, method or function.
- * @deprecated 3.5.0
+ * @deprecated 3.5.0 Use NewProperties instead.
This assumes that NewProperties
is the new class replacing Properties
. Please replace NewProperties
with the actual class if it's different.
@@ -9,6 +9,7 @@ | |||
* Rest API. | |||
* | |||
* @package ga-communicator | |||
* @deprecated 3.5.0 |
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.
The class Accounts
is marked as deprecated. Ensure that all references to this class in the codebase are updated or removed to avoid future issues. If this class is part of a public API, make sure to communicate this change properly to the users and provide them with an alternative if possible.
- * @deprecated 3.5.0
+ * @deprecated 3.5.0 Use NewClass instead.
This comment assumes that NewClass
is the new class replacing Accounts
. Replace NewClass
with the actual class name if it's different.
/** | ||
* Render mea tag. | ||
* | ||
* @since 3.6.0 | ||
* @return void | ||
*/ | ||
public function render_meta() { | ||
$meta_keys = array_filter( array_map( 'trim', explode( ',', $this->setting->get_option( 'meta' ) ) ) ); | ||
$meta_tags = []; | ||
$place_holders = $this->setting->placeholder->get(); | ||
foreach ( $place_holders as $placeholder ) { | ||
if ( in_array( $placeholder['name'], $meta_keys, true ) ) { | ||
if ( isset( $placeholder['callback'] ) && is_callable( $placeholder['callback'] ) ) { | ||
$meta_tags[ 'gacommunicator:' . $placeholder['name'] ] = $placeholder['callback'](); | ||
} | ||
} | ||
} | ||
$meta_tags = apply_filters( 'ga_communicator_meta_tags', $meta_tags ); | ||
if ( ! empty( $meta_tags ) ) { | ||
echo "\n<!-- ga-communicaotr meta-tags -->\n"; | ||
foreach ( $meta_tags as $key => $value ) { | ||
printf( '<meta name="%s" contnet="%s" />' . "\n", $key, $value ); | ||
} | ||
} | ||
} |
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.
There seems to be a typo in the printf
function at line 152. The attribute should be content
, not contnet
. This could lead to incorrect HTML meta tags being generated.
- printf( '<meta name="%s" contnet="%s" />' . "\n", $key, $value );
+ printf( '<meta name="%s" content="%s" />' . "\n", $key, $value );
Also, it's good practice to escape output for security reasons. You can use WordPress's esc_attr()
function to ensure that the values of $key
and $value
are safe for output.
- printf( '<meta name="%s" content="%s" />' . "\n", $key, $value );
+ printf( '<meta name="%s" content="%s" />' . "\n", esc_attr($key), esc_attr($value) );
Lastly, consider checking if $value
is not empty before printing the meta tag. This will prevent empty meta tags from being rendered.
foreach ( $meta_tags as $key => $value ) {
+ if (!empty($value)) {
printf( '<meta name="%s" content="%s" />' . "\n", esc_attr($key), esc_attr($value) );
+ }
}
// Meta tags. | ||
add_settings_field( 'ga-meta', __( 'Meta tags', 'ga-communicator' ), function() use ( $choices ) { | ||
$predefined = $this->get_predefined_option( 'meta' ); | ||
$cur_value = $this->get_option( 'meta', true ); | ||
?> | ||
<textarea name="ga-meta" id="ga-meta" class="widefat" rows="5" placeholder="e.g. post_id, post_type, term"><?php echo esc_textarea( $cur_value ); ?></textarea> | ||
<p class="description"> | ||
<?php | ||
echo wp_kses_post( sprintf( | ||
// translators: %s is link URL. | ||
__( 'These tags are rendered as meta tags for Google Tag Manager. See <a href="%s" target="_blank" rel="noopener noreferrer">our wiki</a> for more details.', 'ga-communicator' ), | ||
'https://github.com/kuno1/ga-communicator/wiki/meta%E3%82%BF%E3%82%B0%E3%81%B8%E3%81%AE%E5%87%BA%E5%8A%9B' | ||
) ); | ||
?> | ||
</p> | ||
<?php if ( $predefined ) : ?> | ||
<p class="description"> | ||
<?php esc_html_e( 'Meta tags are defined programmatically.', 'ga-communicator' ); ?> | ||
</p> | ||
<pre><?php esc_html( $predefined ); ?></pre> | ||
<?php | ||
endif; | ||
}, $this->slug, $tag_section ); |
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.
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.
Files selected (2)
- languages/ga-communicator-ja.po (8)
- languages/ga-communicator.pot (5)
Files not summarized due to errors (2)
- languages/ga-communicator-ja.po (diff tokens exceeds limit)
- languages/ga-communicator.pot (diff tokens exceeds limit)
Review comments generated (4)
- Review: 4
- LGTM: 0
Tips
Chat with
CodeRabbit Bot (@coderabbitai
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitai
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignore
anywhere in the PR description to pause further reviews from the bot.
@@ -2,15 +2,15 @@ | |||
msgid "" | |||
msgstr "" | |||
"Project-Id-Version: Google Analytics Communicator\n" | |||
"POT-Creation-Date: 2023-06-01 15:54+0900\n" | |||
"POT-Creation-Date: 2024-01-11 16:35+0900\n" |
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.
"PO-Revision-Date: 2020-06-12 13:57+0900\n" | ||
"Last-Translator: Takahashi Fumiki <takahashi.fumiki@hametuha.co.jp>\n" | ||
"Language-Team: Takahashi Fumiki <takahashi.fumiki@hametuha.co.jp>\n" | ||
"MIME-Version: 1.0\n" | ||
"Content-Type: text/plain; charset=UTF-8\n" | ||
"Content-Transfer-Encoding: 8bit\n" | ||
"Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\n" | ||
"X-Generator: Poedit 3.2.2\n" | ||
"X-Generator: Poedit 3.3.2\n" |
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.
#: ga-communicator.php:50 | ||
msgid "Composer file is missing. Please run composer install." | ||
msgstr "" | ||
|
||
#: src/Kunoichi/GaCommunicator.php:118 | ||
#: src/Kunoichi/GaCommunicator.php:137 | ||
msgid "" | ||
"Universal Analytics stops collecting data at June 2023. Please consider " | ||
"using Google Analytics V4." | ||
msgstr "" | ||
|
||
#: src/Kunoichi/GaCommunicator/Api/Accounts.php:37 | ||
#: src/Kunoichi/GaCommunicator/Api/Accounts.php:38 |
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.
The line numbers associated with the source code references have been updated. This is important for tracking where in the source code these strings are being used. However, the actual message strings have not been changed.
- #: ga-communicator.php:35
+ #: ga-communicator.php:50
- #: src/Kunoichi/GaCommunicator.php:118
+ #: src/Kunoichi/GaCommunicator.php:137
- #: src/Kunoichi/GaCommunicator/Api/Accounts.php:37
+ #: src/Kunoichi/GaCommunicator/Api/Accounts.php:38
#: src/Kunoichi/GaCommunicator/Api/Profiles.php:47 | ||
msgid "Failed to get profiles. Please check permission." | ||
msgstr "" | ||
|
||
#: src/Kunoichi/GaCommunicator/Api/Properties.php:44 | ||
#: src/Kunoichi/GaCommunicator/Api/Properties.php:45 |
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.
Similar to the previous hunk, the line numbers associated with the source code references have been updated. The actual message strings remain unchanged.
- #: src/Kunoichi/GaCommunicator/Api/Profiles.php:46
+ #: src/Kunoichi/GaCommunicator/Api/Profiles.php:47
- #: src/Kunoichi/GaCommunicator/Api/Properties.php:44
+ #: src/Kunoichi/GaCommunicator/Api/Properties.php:45
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.
Files selected (1)
- src/Kunoichi/GaCommunicator/Utility/ScriptRenderer.php (2)
Review comments generated (2)
- Review: 1
- LGTM: 1
Tips
Chat with
CodeRabbit Bot (@coderabbitai
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitai
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignore
anywhere in the PR description to pause further reviews from the bot.
/** | ||
* Render mea tag. | ||
* | ||
* @since 3.6.0 | ||
* @return void | ||
*/ | ||
public function render_meta() { | ||
$meta_keys = array_filter( array_map( 'trim', explode( ',', $this->setting->get_option( 'meta' ) ) ) ); | ||
$meta_tags = []; | ||
$place_holders = $this->setting->placeholder->get(); | ||
foreach ( $place_holders as $placeholder ) { | ||
if ( in_array( $placeholder['name'], $meta_keys, true ) ) { | ||
if ( isset( $placeholder['callback'] ) && is_callable( $placeholder['callback'] ) ) { | ||
$meta_tags[ 'gacommunicator:' . $placeholder['name'] ] = $placeholder['callback'](); | ||
} | ||
} | ||
} | ||
$meta_tags = apply_filters( 'ga_communicator_meta_tags', $meta_tags ); | ||
if ( ! empty( $meta_tags ) ) { | ||
echo "\n<!-- ga-communicaotr meta-tags -->\n"; | ||
foreach ( $meta_tags as $key => $value ) { | ||
printf( '<meta name="%s" contnet="%s" />' . "\n", esc_attr( $key ), esc_attr( $value ) ); | ||
} | ||
} | ||
} |
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.
There is a typo in the printf
function at line 152. The attribute should be content
, not contnet
. This could lead to incorrect HTML meta tags being generated.
- printf( '<meta name="%s" contnet="%s" />' . "\n", esc_attr( $key ), esc_attr( $value ) );
+ printf( '<meta name="%s" content="%s" />' . "\n", esc_attr( $key ), esc_attr( $value ) );
Also, it's important to note that the render_meta
method does not handle exceptions that might occur during the execution of the callback functions. If any of these callbacks throw an exception, it will cause an unhandled exception error. It would be better to wrap the callback execution in a try-catch block and handle potential errors gracefully.
- $meta_tags[ 'gacommunicator:' . $placeholder['name'] ] = $placeholder['callback']();
+ try {
+ $meta_tags[ 'gacommunicator:' . $placeholder['name'] ] = $placeholder['callback']();
+ } catch (\Exception $e) {
+ error_log('Error executing callback for placeholder: ' . $placeholder['name']);
+ }
This way, if a callback fails, it won't stop the entire script from running and will log the error for further debugging.
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.
Files selected (1)
- src/Kunoichi/GaCommunicator/Utility/ScriptRenderer.php (2)
Review comments generated (2)
- Review: 1
- LGTM: 1
Tips
Chat with
CodeRabbit Bot (@coderabbitai
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitai
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignore
anywhere in the PR description to pause further reviews from the bot.
/** | ||
* Render mea tag. | ||
* | ||
* @since 3.6.0 | ||
* @return void | ||
*/ | ||
public function render_meta() { | ||
$meta_keys = array_filter( array_map( 'trim', explode( ',', $this->setting->get_option( 'meta' ) ) ) ); | ||
$meta_tags = []; | ||
$place_holders = $this->setting->placeholder->get(); | ||
foreach ( $place_holders as $placeholder ) { | ||
if ( in_array( $placeholder['name'], $meta_keys, true ) ) { | ||
if ( isset( $placeholder['callback'] ) && is_callable( $placeholder['callback'] ) ) { | ||
try { | ||
$meta_tags[ 'gacommunicator:' . $placeholder['name'] ] = $placeholder['callback'](); | ||
} catch ( \Exception $e ) { | ||
error_log( 'Error while executing: ' .$e->getMessage() ); | ||
} | ||
} | ||
} | ||
} | ||
$meta_tags = apply_filters( 'ga_communicator_meta_tags', $meta_tags ); | ||
if ( ! empty( $meta_tags ) ) { | ||
echo "\n<!-- ga-communicaotr meta-tags -->\n"; | ||
foreach ( $meta_tags as $key => $value ) { | ||
printf( '<meta name="%s" content="%s" />' . "\n", esc_attr( $key ), esc_attr( $value ) ); | ||
} | ||
} | ||
} | ||
|
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.
The render_meta
function seems to be well implemented. However, there's a potential issue with the error handling. The current implementation logs an error message when an exception is thrown but continues execution. This could lead to unexpected behavior if the callback functions are critical for the correct generation of meta tags. Consider rethrowing the exception after logging it or handle it in a way that doesn't affect the rest of the execution.
try {
$meta_tags[ 'gacommunicator:' . $placeholder['name'] ] = $placeholder['callback']();
} catch ( \Exception $e ) {
error_log( 'Error while executing: ' .$e->getMessage() );
+ throw $e;
}
Also, consider sanitizing the $key
and $value
variables before using them in the printf
function at line 156. Although you're using esc_attr
which is good, it might not be enough depending on the source of these values. If they come from user input or an untrusted source, additional validation or sanitization might be necessary to prevent potential security issues like XSS attacks.
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.
Files selected (1)
- src/Kunoichi/GaCommunicator/Utility/ScriptRenderer.php (2)
Review comments generated (2)
- Review: 1
- LGTM: 1
Tips
Chat with
CodeRabbit Bot (@coderabbitai
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitai
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignore
anywhere in the PR description to pause further reviews from the bot.
/** | ||
* Render mea tag. | ||
* | ||
* @since 3.6.0 | ||
* @return void | ||
*/ | ||
public function render_meta() { | ||
$meta_keys = array_filter( array_map( 'trim', explode( ',', $this->setting->get_option( 'meta' ) ) ) ); | ||
$meta_tags = []; | ||
$place_holders = $this->setting->placeholder->get(); | ||
foreach ( $place_holders as $placeholder ) { | ||
if ( in_array( $placeholder['name'], $meta_keys, true ) ) { | ||
if ( isset( $placeholder['callback'] ) && is_callable( $placeholder['callback'] ) ) { | ||
try { | ||
$meta_tags[ 'gacommunicator:' . $placeholder['name'] ] = $placeholder['callback'](); | ||
} catch ( \Exception $e ) { | ||
error_log( 'Error while executing: ' . $e->getMessage() ); | ||
} | ||
} | ||
} | ||
} | ||
$meta_tags = apply_filters( 'ga_communicator_meta_tags', $meta_tags ); | ||
if ( ! empty( $meta_tags ) ) { | ||
echo "\n<!-- ga-communicaotr meta-tags -->\n"; | ||
foreach ( $meta_tags as $key => $value ) { | ||
printf( '<meta name="%s" content="%s" />' . "\n", esc_attr( $key ), esc_attr( $value ) ); | ||
} | ||
} | ||
} | ||
|
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.
The render_meta
function seems to be well implemented. However, there is a potential issue with error handling. When an exception occurs in the callback function, it's caught and logged, but the execution continues as if nothing happened. This could lead to unexpected behavior if the exception was due to a serious problem that should have halted the execution. Consider rethrowing the exception after logging it or at least return from the function to prevent further execution.
try {
$meta_tags[ 'gacommunicator:' . $placeholder['name'] ] = $placeholder['callback']();
} catch ( \Exception $e ) {
error_log( 'Error while executing: ' . $e->getMessage() );
+ throw $e;
}
Also, consider sanitizing the meta keys before using them. Even though they are coming from your own settings, it's a good practice to sanitize all data before outputting it to prevent any potential XSS attacks.
foreach ( $meta_tags as $key => $value ) {
- printf( '<meta name="%s" content="%s" />' . "\n", esc_attr( $key ), esc_attr( $value ) );
+ printf( '<meta name="%s" content="%s" />' . "\n", sanitize_key( $key ), esc_attr( $value ) );
}
closes #55
Summary by CodeRabbit
ga-custom-dimensions.js
file andAccounts
,Profiles
, andProperties
classes in thega-communicator
package.render_meta()
in theScriptRenderer
class to render meta tags based on placeholder values.