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 Google Analytics support #1058

Merged
merged 4 commits into from Aug 3, 2019

Conversation

@DanielBaulig
Copy link
Contributor

commented Jul 25, 2019

These commits add support for Google Analytics.

A Google Analytics tracking ID can be set from the Podlove Publisher tracking settings.
If provided, Podlove Publisher will forward available tracking data to Google Analytics.

DanielBaulig added some commits Jul 24, 2019

Send download intents to GA when a GA ID is set
Tracks download intents using Google Analytics when a Google Analytics Tracking ID was set in the settings page.
Cleanup
Remove unused ip_string param.
@eteubert
Copy link
Member

left a comment

Looks good! Just a few minor notes, see inline remarks.

// Pageview params
't' => 'pageview', // hit type
'dh' => $_SERVER[HTTP_HOST], // document host

This comment has been minimized.

Copy link
@eteubert

eteubert Jul 25, 2019

Member

missing quotes here and on next line

// This way all Podlove sources can be easily grouped into GA Channels.
'cs' => $ptm_context, // campaign source
'cm' => $ptm_source, // campaign medium
'ci' => $media_file->episode()->number, // campaign id

This comment has been minimized.

Copy link
@eteubert

eteubert Jul 25, 2019

Member

Performance concern: do $episode = $media_file->episode() once and use that instead of calling $media_file->episode() repeatedly as it does duplicate database requests each time.

}
\Podlove\Log::get()->addError('GA Measurement Protocol request failed.');
} else {
\Podlove\Log::get()->addInfo('GA Measurement Protocol request successful: ' . $body);

This comment has been minimized.

Copy link
@eteubert

eteubert Jul 25, 2019

Member

I would prefer addDebug in all success cases so the log isn't flooded with tracking requests.

@DanielBaulig

This comment has been minimized.

Copy link
Contributor Author

commented Jul 28, 2019

@eteubert Thanks for the feedback. Should be addressed in recent commit.

@eteubert eteubert merged commit 262851f into podlove:master Aug 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.