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

Widgets: Record events for the Simple Payments widget #9803

Merged
merged 6 commits into from
Jun 26, 2018

Conversation

Copons
Copy link
Contributor

@Copons Copons commented Jun 19, 2018

Fixes Automattic/wp-calypso#25484

Add metrics for the Simple Payments widget.

Changes proposed in this Pull Request:

  • Add three new Tracks event:
    • jetpack_wpa_simple_payments_button_create
    • jetpack_wpa_simple_payments_button_update
    • jetpack_wpa_simple_payments_button_delete

The extra event properties are the same as in Calypso: id, price, and currency for create and update; only id for delete.

  • Add three new MC stats to the simple_payments group:
    • button_created
    • button_updated
    • button_deleted

Testing instructions:

I'm not sure how to properly test this on WPCOM while it's still a PR to be merged into a feature branch.

@Copons
Copy link
Contributor Author

Copons commented Jun 19, 2018

@Automattic/jetpack-crew 👋
Pinging you informally to check with you that I'm doing the right thing here.

Also, we should probably also track when we insert or remove the Simple Payments widget, but I'm wondering if we already track that "globally" somehow.

And another doubt: our analytics lib forces us to prepend jetpack_ to the event names.
Am I supposed to do anything different for the WPCOM side?

@gwwar You mentioned a simple_payments_dialog_edit_view or simple_payments_dialog_customize_view.
Do you mean that we should track whenever a user opens the edit form of a SP in the Customizer?
It's bit overkill maybe? I've added a track/bump for when the user actually updates the button (not the widget), and I think it should be enough.

@gwwar
Copy link
Contributor

gwwar commented Jun 19, 2018

Do you mean that we should track whenever a user opens the edit form of a SP in the Customizer?

Not sure if it'd be too difficult to track, but I think the initial click on the blue edit pencil (or initial load of the form) in the customizer would be interesting. Though I imagine we're smart enough to de-dupe events for users on a single session.

@gwwar gwwar requested a review from a team June 19, 2018 19:50
@Copons Copons changed the title Record events for buttons created, updated, and deleted via the widget Widgets: Record events for the Simple Payments widget Jun 19, 2018
@Copons
Copy link
Contributor Author

Copons commented Jun 19, 2018

@gwwar I think that clicking the blue pencils in the preview or opening a widget via the menu, those are (should be?) tracked independently of their widget.
It's unclear to me how to see events recorded in JP, so I've got a bit of trouble figuring out if they actually record such events and if they do what's their name, but I'll look into it ASAP.

In other words, if the pencil doesn't record any events, we (or maybe the JP folks? 😛) should take care of that on a higher level, and not only for this widget.

@vindl vindl added this to the 6.3 milestone Jun 20, 2018
@brbrr brbrr added the [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it label Jun 20, 2018
@matticbot

This comment has been minimized.

@matticbot
Copy link
Contributor

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

@Copons Copons force-pushed the add/simple-payments-metrics branch 2 times, most recently from 9ee1682 to 81090c3 Compare June 21, 2018 10:57
@Copons Copons added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] In Progress labels Jun 21, 2018
@rodrigoi rodrigoi force-pushed the add/simple-payments-widget branch 2 times, most recently from 20f0b95 to f9a214c Compare June 22, 2018 18:41
Copy link
Contributor

@gwwar gwwar left a comment

Choose a reason for hiding this comment

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

Thanks @Copons I think this looks correct. I did some light smoke testing and checked to see if the jetpack tracks event appeared. I'll of course defer to any Jetpack convention and if we need any other considerations for GDPR.

@Copons Copons force-pushed the add/simple-payments-metrics branch from 81090c3 to f7b9338 Compare June 25, 2018 15:27
@Copons
Copy link
Contributor Author

Copons commented Jun 25, 2018

Rebased and ready to go.

@pesieminski re: GDPR
The bot raised a concern about a function we're not using anymore in this PR.
Though, we've replaced it with another function (jetpack_tracks_record_event) that takes a user object (obtained with wp_get_current_user()) as parameter.
What exactly should we be careful with here?

@Copons
Copy link
Contributor Author

Copons commented Jun 26, 2018

A bit of an explanation of the changes in a9661dd:

  • Replaced if ( IS_WPCOM ) with if ( function_exists( 'bump_stats_extras' ) ) because the latter (and jetpack_bump_stats_extra as well) only exists on WPCOM. So the outcome is the same, but it's a little bit more elegant.

  • Since there is no bump_stats_extra in JP proper, I've adopted an established workaround: start tracking with $jetpack->stat() and immediately do a one-off stat bump with $jetpack->do_stats( 'server_side' );.

@Copons Copons merged this pull request into add/simple-payments-widget Jun 26, 2018
@Copons Copons deleted the add/simple-payments-metrics branch June 26, 2018 14:36
oskosk pushed a commit that referenced this pull request Jun 26, 2018
* Widgets: Adds support for Simple Payment Buttons as Widgets

* Simple Payments Widget: Add style overrides (#9580)

Override the media query and ensure that Simple Payments widgets are always displayed as a single column.

* Widgets: Only render the Simple Payments widget if its button exists (#9673)

In the frontend, only show the widget if the Simple Payments shortcode is parsed successfully.

In the customizer, show the widget regardless, so that it can be modified via the pencil icon.

* Simple Payment Widget: Manage Products from the Customizer (#9699)

* Customizer: Simple Payments Widget breaks when starting without products (#9809)

* Widgets: Hide Simple Payments create and edit buttons in widgets.php (#9811)

* Widgets: Record events for the Simple Payments widget (#9803)

* Widgets: Add Plan Check to the Simple Payments Widget (#9824)

* Customizer: keep Simple Payments Widget Customizer in sync between instances (#9814)

* Customizer: improves price validation on the Simple Payments Widget Customizer

* Customizer: improves the behaviour of the action buttons on the Simple Payments Widget Customizer
@kraftbj kraftbj removed the [Status] Needs Review To request a review from Crew. Label will be renamed soon. label Sep 29, 2021
@jeherve jeherve added the [Feature] Pay with PayPal aka Simple Payments label Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Extra Sidebar Widgets [Feature] Pay with PayPal aka Simple Payments [Pri] BLOCKER Touches WP.com Files [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants