[Simple Analytics] Add Data Layer & Form Event Tracking#204
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new “Simple Analytics” data layer to track Mailchimp signup form views/submissions and surfaces the data in the WP Admin Analytics page.
Changes:
- Adds a custom DB table (
{prefix}_mailchimp_sf_form_analytics) plus query/increment helpers and an AJAX handler for form-view tracking. - Emits a new
mailchimp_sf_form_submission_successaction on successful submissions and hooks it to increment submission counts. - Adds
data-list-idto rendered forms and adds frontend JS to POST form-view events.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
mailchimp_widget.php |
Adds data-list-id attribute to widget/shortcode form markup for JS tracking. |
includes/blocks/mailchimp/markup.php |
Adds data-list-id attribute to block form markup for JS tracking. |
assets/js/mailchimp.js |
Adds client-side view tracking that POSTs to admin-ajax.php. |
includes/class-mailchimp-form-submission.php |
Fires a new action on successful form submission for analytics hooks. |
includes/class-mailchimp-analytics-data.php |
New analytics storage/query class, table creation, AJAX handler, and submission tracking hook. |
mailchimp.php |
Loads the analytics data class, registers activation hook, localizes AJAX URL + nonce. |
mailchimp_upgrade.php |
Runs table creation in upgrade routine based on a stored DB version option. |
includes/class-mailchimp-analytics.php |
Removes trailing whitespace only. |
includes/admin/templates/analytics.php |
Fetches analytics data and currently outputs it via print_r() for display. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public function init() { | ||
| add_action( 'wp_ajax_mailchimp_sf_track_form_view', array( $this, 'handle_form_view' ) ); | ||
| add_action( 'wp_ajax_nopriv_mailchimp_sf_track_form_view', array( $this, 'handle_form_view' ) ); | ||
| add_action( 'mailchimp_sf_form_submission_success', array( $this, 'track_submission' ) ); | ||
| } |
There was a problem hiding this comment.
This PR introduces new analytics behaviors (AJAX endpoint for view tracking, DB writes, and a new submission-success hook) but doesn’t add/extend automated coverage. There is an existing Cypress suite under tests/cypress; adding at least one e2e test that verifies view tracking + submission tracking increments the expected counts would help prevent regressions.
iamdharmesh
left a comment
There was a problem hiding this comment.
Thanks @alaca. Overall this looks good and added few feedback/comment. Could you please check?
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Dharmesh Patel <dspatel44@gmail.com>
Co-authored-by: Dharmesh Patel <dspatel44@gmail.com>
…nd-form-event-tracking
|
@iamdharmesh resolved. Also, the data is now loaded via ajax for future integrations with chart.js |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
includes/admin/templates/analytics.php:110
- There are two elements with the same id
mailchimp-sf-analytics-content(one empty and one containing the placeholder). Duplicate IDs produce invalid markup andgetElementById()will return only the first element, leaving the placeholder orphaned/unreachable. Keep a single container and place the placeholder inside it (or remove the duplicate).
<div class="mailchimp-sf-analytics-content" id="mailchimp-sf-analytics-content">
</div>
<div class="mailchimp-sf-analytics-content" id="mailchimp-sf-analytics-content">
<div class="mailchimp-sf-analytics-placeholder">
<p><?php esc_html_e( 'Select a date range and list to view analytics.', 'mailchimp' ); ?></p>
</div>
</div>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| const { data } = response; | ||
| contentArea.innerHTML = JSON.stringify(data); |
There was a problem hiding this comment.
contentArea.innerHTML = JSON.stringify(data) uses innerHTML with server-provided content, which is unnecessary and can introduce XSS risks if the payload ever contains user-controlled strings. Prefer assigning to textContent for raw JSON output, or render into DOM nodes with proper escaping.
| contentArea.innerHTML = JSON.stringify(data); | |
| contentArea.textContent = JSON.stringify(data); |
| ); | ||
| // phpcs:enable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, WordPress.DB.PreparedSQL.InterpolatedNotPrepared | ||
|
|
||
| if ( false === $result ) { |
There was a problem hiding this comment.
This error_log() call will likely violate the WordPress PHPCS sniff WordPress.PHP.DevelopmentFunctions.error_log_error_log (other files use // phpcs:ignore when logging). Either add the appropriate PHPCS ignore/comment, or route logging through the existing logging mechanism/filter used elsewhere in the plugin.
| if ( false === $result ) { | |
| if ( false === $result ) { | |
| // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log -- Logs DB write failures for analytics tracking. |
| ); | ||
| // phpcs:enable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, WordPress.DB.PreparedSQL.InterpolatedNotPrepared | ||
|
|
||
| if ( false === $result ) { |
There was a problem hiding this comment.
This error_log() call will likely violate the WordPress PHPCS sniff WordPress.PHP.DevelopmentFunctions.error_log_error_log (other files use // phpcs:ignore when logging). Either add the appropriate PHPCS ignore/comment, or route logging through the existing logging mechanism/filter used elsewhere in the plugin.
| if ( false === $result ) { | |
| if ( false === $result ) { | |
| // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log -- Intentional diagnostic logging for database write failures. |
| public function init() { | ||
| add_action( 'wp_ajax_mailchimp_sf_track_form_view', array( $this, 'handle_form_view' ) ); | ||
| add_action( 'wp_ajax_nopriv_mailchimp_sf_track_form_view', array( $this, 'handle_form_view' ) ); | ||
| add_action( 'wp_ajax_mailchimp_sf_get_analytics', array( $this, 'handle_get_analytics' ) ); | ||
| add_action( 'mailchimp_sf_form_submission_success', array( $this, 'track_submission' ) ); | ||
| } |
There was a problem hiding this comment.
New analytics behavior (AJAX endpoints + view/submission tracking) isn’t covered by existing Cypress tests. Since the repo already has e2e coverage for the Analytics admin page, add tests that (1) trigger a form view POST and assert the analytics table/counts change, and (2) submit a form successfully and assert submissions increment (and invalid submissions do not).
| // Create analytics table if it doesn't exist. | ||
| $analytics_db_version = get_option( 'mailchimp_sf_analytics_db_version' ); | ||
| if ( false === $analytics_db_version || version_compare( Mailchimp_Analytics_Data::DB_VERSION, $analytics_db_version, '>' ) ) { | ||
| Mailchimp_Analytics_Data::create_table(); | ||
| } |
There was a problem hiding this comment.
This table-creation block won’t run if mailchimp_version_check() returns early when MCSF_VER === get_option('mc_version'). That means a site can end up with no analytics table if the mc_version option is already current (e.g., version string unchanged, or option manually set) but mailchimp_sf_analytics_db_version is missing/outdated. Consider moving the analytics DB version check above the early return, or adjusting the early return condition to also account for the analytics DB version option.
iamdharmesh
left a comment
There was a problem hiding this comment.
Thanks for the changes @alaca. This looks good overall now. Just added 1 minor nit comment and this will be ready for the go. Thanks
iamdharmesh
left a comment
There was a problem hiding this comment.
Thanks for changes @alaca
QA Status: Query Raised
|
|
@ankitguptaindia yeah, I agree, but I checked the PRD doc, and I was not able to find anything related to this. We can always address this later. |
…nd-form-event-tracking
QA Status: Approved ✅Test Result:Tested the analytics tracking functionality and confirmed the analytics table is created successfully on plugin activation. Verified that form views and successful submissions are tracked correctly, including multiple forms on the same page, while invalid submissions are not tracked. Also confirmed analytics data displays correctly under Functional Demo / Screencast:Recording.1793.mp4Additional enhancement suggestion logged separately: #215🖥 Testing Environment - Details
|

Description of the Change
fires an AJAX POST on page load, deduplicated per list_id
Closes #198
How to test the Change
wp_mailchimp_sf_form_analyticstable is createdlist_idandtoday's date
Changelog Entry
Credits
Props @alaca
Checklist: