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

Refactor JS snippet to use WP AJAX #109

Merged
merged 12 commits into from
Apr 13, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# Changelog
All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/).

## Unreleased
* Refactored JavaScript tracking to use WP AJAX

## 1.6.3
* Fix compatibility issue with some PHP implementations not populating `INPUT_SERVER`
* Fix failing blacklist check for empty referrers
Expand Down
92 changes: 57 additions & 35 deletions inc/class-statify-frontend.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,42 +22,42 @@ class Statify_Frontend extends Statify {
* Track the page view
*
* @since 0.1.0
* @version 1.4.2
* @since 1.7.0 $is_snippet parameter added.
* @version 1.7.0
*
* @return bool
* @param boolean $is_snippet Is tracking triggered via JS (default: false).
*
* @return boolean
*/
public static function track_visit() {
public static function track_visit( $is_snippet = false ) {

/* Init vars */
// Check of JS snippet is configured.
$use_snippet = self::$_options['snippet'];
$is_snippet = $use_snippet && get_query_var( 'statify_target' );

/* Set target & referrer */
if ( $is_snippet ) {
$target = urldecode( get_query_var( 'statify_target' ) );
$referrer = urldecode( get_query_var( 'statify_referrer' ) );
// Set target & referrer.
if ( $use_snippet && $is_snippet ) {
$target = urldecode( isset( $_REQUEST['statify_target'] ) ? wp_unslash( $_REQUEST['statify_target'] ) : '/' );
$referrer = urldecode( isset( $_REQUEST['statify_referrer'] ) ? wp_unslash( $_REQUEST['statify_referrer'] ) : '' );
} elseif ( ! $use_snippet ) {
$target = filter_var(
( isset( $_SERVER['REQUEST_URI'] ) ? wp_unslash( $_SERVER['REQUEST_URI'] ) : '/' ),
FILTER_SANITIZE_URL
);
if ( is_null( $target ) || false === $target ) {
$target = '/';
} else {
$target = wp_unslash( $target );
}

$referrer = filter_var(
( isset( $_SERVER['HTTP_REFERER'] ) ? wp_unslash( $_SERVER['HTTP_REFERER'] ) : '' ),
FILTER_SANITIZE_URL
);
if ( is_null( $referrer ) || false === $referrer ) {
$referrer = '';
}
$target = isset( $_SERVER['REQUEST_URI'] ) ? wp_unslash( $_SERVER['REQUEST_URI'] ) : '/';
$referrer = isset( $_SERVER['HTTP_REFERER'] ) ? wp_unslash( $_SERVER['HTTP_REFERER'] ) : '';
} else {
return false;
}

// Sanitize.
$target = filter_var( $target, FILTER_SANITIZE_URL );
if ( is_null( $target ) || false === $target ) {
$target = '/';
} else {
stklcode marked this conversation as resolved.
Show resolved Hide resolved
$target = wp_unslash( $target );
}

$referrer = filter_var( $referrer, FILTER_SANITIZE_URL );
if ( is_null( $referrer ) || false === $referrer ) {
$referrer = '';
}

/* Invalid target? */
if ( empty( $target ) || ! wp_validate_redirect( $target, false ) ) {
return self::_jump_out( $is_snippet );
Expand Down Expand Up @@ -116,6 +116,20 @@ public static function track_visit() {
return self::_jump_out( $is_snippet );
}

/**
* Track the page view via AJAX.
*
* @return void
Copy link
Member

Choose a reason for hiding this comment

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

In my mind there is not helpful. Also the hint on phpdoc - This tag should not be used for constructors or methods defined with a void return type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're using this tag in 15 places... Apparently all introduced by myself (default pattern in my IDE because it's mandatory in most of my styleguides - and btw. the PHPdoc ref marks this as "MAY be omitted").

We shouldn't mix it up, so either remove all or none.
Along with some int(eger) and bool(ean) tags probably little out of scope of this PR.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, plugincollektiv need a style guide, simplest way is the WP codex. In this context it is only a hint and the result of different views, works outside of the pluginkollektiv projects. Feel free to leave that or remove all ;)

*/
public static function track_visit_ajax() {
// Check AJAX referrer.
check_ajax_referer( 'statify_track' );
// Only do something if snippet use is actually configured.
if ( self::$_options['snippet'] ) {
self::track_visit( true );
}
}

/**
* Find the position of the first occurrence of a substring in a string about a array.
*
Expand Down Expand Up @@ -298,23 +312,31 @@ public static function query_vars( $vars ) {
*/
public static function wp_footer() {

/* Skip by option */
// Skip by option.
if ( ! self::$_options['snippet'] ) {
return;
}

/* Skip by internal rules (#84) */
// Skip by internal rules (#84).
if ( self::_is_internal() ) {
return;
}

/* Load template */
load_template(
wp_normalize_path(
sprintf(
'%s/views/js-snippet.php',
STATIFY_DIR
)
wp_enqueue_script(
'statify-js',
plugins_url( 'js/snippet.js', STATIFY_FILE ),
array(),
STATIFY_VERSION,
true
);

// Add endpoint to script.
wp_localize_script(
'statify-js',
'statify_ajax',
array(
'url' => admin_url( 'admin-ajax.php' ),
'nonce' => wp_create_nonce( 'statify_track' ),
)
);
}
Expand Down
8 changes: 5 additions & 3 deletions inc/class-statify.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ public static function instance() {
* @version 2017-01-10
*/
public function __construct() {
// Skip me!
if ( ( defined( 'DOING_AUTOSAVE' ) && DOING_AUTOSAVE ) || ( defined( 'DOING_AJAX' ) && DOING_AJAX ) ) {
// Nothing to do on autosave.
if ( defined( 'DOING_AUTOSAVE' ) && DOING_AUTOSAVE ) {
return;
}

Expand All @@ -64,7 +64,9 @@ public function __construct() {
)
);

if ( defined( 'XMLRPC_REQUEST' ) && XMLRPC_REQUEST ) { // XMLRPC.
if ( defined( 'DOING_AJAX' ) && DOING_AJAX ) {
add_action( 'wp_ajax_nopriv_statify_track', array( 'Statify_Frontend', 'track_visit_ajax' ) );
} elseif ( defined( 'XMLRPC_REQUEST' ) && XMLRPC_REQUEST ) { // XMLRPC.
add_filter( 'xmlrpc_methods', array( 'Statify_XMLRPC', 'xmlrpc_methods' ) );
} elseif ( defined( 'DOING_CRON' ) && DOING_CRON ) { // Cron.
add_action( 'statify_cleanup', array( 'Statify_Cron', 'cleanup_data' ) );
Expand Down
20 changes: 11 additions & 9 deletions js/snippet.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
try {
const statifyReq = new XMLHttpRequest();
statifyReq.open(
'GET',
document.getElementById('statify-js-snippet').getAttribute('data-home-url')
+ '?statify_referrer=' + encodeURIComponent(document.referrer)
+ '&statify_target=' + encodeURIComponent(location.pathname + location.search)
);
statifyReq.send(null);
} catch (e) {
jQuery.ajax( {
stklcode marked this conversation as resolved.
Show resolved Hide resolved
type: 'POST',
url : statify_ajax.url,
data: {
_ajax_nonce : statify_ajax.nonce,
action : 'statify_track',
statify_referrer: encodeURIComponent( document.referrer ),
statify_target : encodeURIComponent( location.pathname + location.search )
}
} );
} catch ( e ) {
}
1 change: 1 addition & 0 deletions statify.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
define( 'STATIFY_FILE', __FILE__ );
define( 'STATIFY_DIR', dirname( __FILE__ ) );
define( 'STATIFY_BASE', plugin_basename( __FILE__ ) );
define( 'STATIFY_VERSION', '1.6.3' );
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be 1.7.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely (if we don‘t add this to 1.6.x - and I don‘t think so - or postpone this to 1.8 obviously)



/* Hooks */
Expand Down
21 changes: 0 additions & 21 deletions views/js-snippet.php

This file was deleted.