Add tracking for the contact form block by scruffian · Pull Request #13688 · Automattic/jetpack

scruffian · 2019-10-08T11:11:07Z

Right now we can track how many contact forms are submitted, but it's not possible to distinguish which ones are from the shortcode and which ones are from the block. This adds a tracks event specifically for the block, so that we can track it independently of the shortcode.

Changes proposed in this Pull Request:

Relies on the hasFormSettingsSet attribute which is set by the block
When this attribute exists we add a hidden field to the form with the name is_block
If this hidden field is submitted we then add a value to the extra_values array
If is_block is set in the extra_values array, then send a jetpack_contact_form_gutenberg_block_message_sent event to Tracks

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Adding features to Jetpack

Testing instructions:

Create a new post and add a contact form block
From the front end of your site, submit the contact form
Go to Tracks and search for the event jetpack_contact_form_gutenberg_block_message_sent
Now create another post and add the contact form shortcode
From the front end of your site, submit the contact form
Go to Tracks and search for the event jetpack_contact_form_gutenberg_block_message_sent - you should not see it.

matticbot · 2019-10-08T11:11:13Z

Caution: This PR has changes that must be merged to WordPress.com
Hello scruffian! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D33693-code before merging this PR. Thank you!

jetpackbot · 2019-10-08T11:13:23Z

	Warnings
⚠️	"Proposed changelog entry" is missing for this PR. Please include any meaningful changes

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 012227f

jeherve

All tests are currently failing, so they'll need to be updated:

WP_Test_Grunion_Contact_Form::test_process_submission_will_store_a_feedback_correctly_with_default_form
Undefined index: is_block

Are those events triggered even when the site is in development mode, when the site owner has not given their consent for tracking yet?

modules/contact-form/tracks-events.php

modules/contact-form/grunion-contact-form.php

modules/contact-form/tracks-events.php

scruffian · 2019-10-09T14:26:39Z

Are those events triggered even when the site is in development mode, when the site owner has not given their consent for tracking yet?

The contact form won't work unless Jetpack is connected, so we are safe in that respect.

jeherve · 2019-10-09T14:28:20Z

The contact form won't work unless Jetpack is connected

Jetpack's Contact Form is one of the features that is accessible when Jetpack is in development mode, so we do need to take it into account imo.

matticbot · 2019-10-09T14:32:13Z

scruffian, Your synced wpcom patch D33693-code has been updated.

matticbot · 2019-10-09T15:06:20Z

scruffian, Your synced wpcom patch D33693-code has been updated.

modules/contact-form/tracks-events.php

matticbot · 2019-10-10T11:59:42Z

scruffian, Your synced wpcom patch D33693-code has been updated.

jeherve · 2019-10-10T16:12:58Z

Ongoing discussions about this here:
p1570722249055600-slack-jetpack-crew

…utenberg blocks

If we don't do that we can't separate the 2 when looking at the data later on.

jeherve · 2019-10-11T16:20:19Z

This should now be ready for a new review. The WordPress.com diff is also updated.

We now include the blog ID when possible, to get valid Tracks events.
We now fallback to the Jetpack Master User information when a contact form is submitted from a non-logged in user.
We now have a different prefix for the event name that will be fired, depending on the platform. This will allow us to differentiate data coming from WordPress.com sites vs. Jetpack sites.

scruffian · 2019-10-11T20:54:46Z

Thanks @jeherve these changes look great.

The only one I am not sure about is:

We now have a different prefix for the event name that will be fired, depending on the platform. This will allow us to differentiate data coming from WordPress.com sites vs. Jetpack sites.

I worry that this might make it harder to get overall number for contact form submissions. @huguesvincent, what do you think?

huguesvincent · 2019-10-15T09:23:10Z

Would it allow us to get the volume on DotCom alone?
That's really what I was hoping to get here. I don't think we'll be considering to do any sort of tiring or carve a different experience depending on the volume you get. We might on DotCom particularly as ZBS integration comes along.

jeherve · 2019-10-15T09:54:21Z

Would it allow us to get the volume on DotCom alone?

Yes, you'd be able to track both Jetpack usage and WordPress.com usage separately, or together with a %_contact_form_block_message_sent wildcard.

scruffian · 2019-10-15T11:08:21Z

or together with a %_contact_form_block_message_sent wildcard.

Oh I didn't know about this. That's great 👍

Changes were made to address feedback.

jeherve

This works well in my tests, here and on wpcom, but I'd appreciate another review before merge since I contributed to this PR.

Thank you!

modules/contact-form/grunion-contact-form.php

See https://github.com/Automattic/jetpack/pull/13688/files#r339803539

matticbot · 2019-10-28T22:13:19Z

This PR looks like it might contain user tracking functions. We need to make sure that it is GDPR Compliant.

Rules triggering this positive scan:

Called "record_user_event" function.

cc: @pesieminski

jeherve

This seems to work well in my tests. Merging.

* 7.9: Changelog * Update version number * Update stable tag and tested up to * Changelog: add #13530 * changelog: add #13578 * Changelog: add #13598 * Changelog: add entry for numerous block preview changes * Changelog: add #13599 * changelog: add #13541 * Changelog: add #13542 * Changelog: add #13331 * Changelog: add #13558 * Changelog: add #13409 * Changelog: add #13582 * Changelog: add #13600 * Changelog: add #13601 * Changelog: add #13595 * Changelog: add #12695 * Changelog: add #13009 * Changelog: add #13649 * Changelog: add #13450 * Changelog: add #13507 * Changelog: add #13658 * Changelog: add #13687 * changelog: add #13683 * Changelog: add #9323 * Changelog: add #13681 * Fix typos in readme * Add link to WordPress Beta Tester plugin * Changelog: add #13630 * Changelog: add #13695 * Changelog: add #13659 * Changelog: add #13716 * Changelog: add #13664 * Changelog: add #13682 * Changelog: add #13362 * Changelog: add #13563 * Add testing list for #13563 * Changelog: add #13735 * Changelog: add #13752 * Changelog: add #13624 * Changelog: add #13756 * Changelog: add #13745 * Changelog: add #13728 * Changelog: add #13779 * Changelog: add #13699 * Changelog: add #13804 * Changelog: add #13761 * Changelog: add #13637 * Changelog: add #13517 * Changelog: add #13521 * Changelog: add #13729 * Testing list: add testing instructions for #13729 * Changelog: add sync changes * Changelog: add #13807 * Changelog: add #13654 * Changelog: add #13795 * Changelog: add #13801 * Changelog: add #13818 * Changelog: add #13725 * Changelog: add #13831 * Changelog: add #13516 * Testing list: add Twenty Twenty instructions * Changelog: add #13799 * Changelog: add #13805 * Changelog: add #13688 * Changelog: add #13830

scruffian added the [Feature] Forms label Oct 8, 2019

scruffian requested a review from a team October 8, 2019 11:11

scruffian self-assigned this Oct 8, 2019

matticbot added the Touches WP.com Files label Oct 8, 2019

scruffian added the [Status] Needs Review This PR is ready for review. label Oct 8, 2019

jeherve added Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Tracks labels Oct 8, 2019

jeherve previously requested changes Oct 8, 2019

View reviewed changes

jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Oct 8, 2019

scruffian commented Oct 9, 2019

View reviewed changes

modules/contact-form/tracks-events.php Outdated Show resolved Hide resolved

scruffian mentioned this pull request Oct 9, 2019

Tracks: Don't track users in dev mode or when opted out #13698

Merged

jeherve added [Status] In Progress and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Oct 10, 2019

scruffian added 8 commits October 11, 2019 16:15

Add tracking for the contact form

b57d676

Add file

246ec95

Make it possible to track contact form submissions only coming from G…

48d0f73

…utenberg blocks

update event name

3693d25

escape data sent to tracks

73ebfee

add a jetpack prefix to the tracks event function

2eabc21

add new file to the phpcs whitelist

62e8cd4

fix undefined index error

934284f

Use a different prefix for WordPress.com and Jetpack

a641946

If we don't do that we can't separate the 2 when looking at the data later on.

jeherve added this to the 7.9 milestone Oct 11, 2019

jeherve added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Oct 11, 2019

Add missing comment *

49d904d

jeherve previously approved these changes Oct 25, 2019

View reviewed changes

jeherve self-assigned this Oct 25, 2019

dereksmart reviewed Oct 28, 2019

View reviewed changes

modules/contact-form/grunion-contact-form.php Outdated Show resolved Hide resolved

Switch to using record_user_event

012227f

See https://github.com/Automattic/jetpack/pull/13688/files#r339803539

jeherve dismissed their stale review via 012227f October 28, 2019 22:13

matticbot added the [Status] Needs GDPR Review label Oct 28, 2019

jeherve removed the [Status] Needs GDPR Review label Oct 28, 2019

jeherve approved these changes Oct 29, 2019

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Oct 29, 2019

jeherve merged commit c2dd31d into master Oct 29, 2019

jeherve deleted the add/contact-form-tracking branch October 29, 2019 11:22

matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Oct 29, 2019

jeherve added [Status] Has Changelog and removed [Status] Needs Changelog labels Oct 29, 2019

jeherve added a commit that referenced this pull request Oct 29, 2019

Changelog: add #13688

3a8825b

Conversation

scruffian commented Oct 8, 2019

Changes proposed in this Pull Request:

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Testing instructions:

Uh oh!

matticbot commented Oct 8, 2019

Uh oh!

jetpackbot commented Oct 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

scruffian commented Oct 9, 2019

Uh oh!

jeherve commented Oct 9, 2019

Uh oh!

matticbot commented Oct 9, 2019

Uh oh!

matticbot commented Oct 9, 2019

Uh oh!

Uh oh!

matticbot commented Oct 10, 2019

Uh oh!

jeherve commented Oct 10, 2019

Uh oh!

jeherve commented Oct 11, 2019

Uh oh!

scruffian commented Oct 11, 2019

Uh oh!

huguesvincent commented Oct 15, 2019

Uh oh!

jeherve commented Oct 15, 2019

Uh oh!

scruffian commented Oct 15, 2019

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

matticbot commented Oct 28, 2019

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jetpackbot commented Oct 8, 2019 •

edited

Loading