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

Create process telemetry rest route #1684

Merged
merged 12 commits into from
Dec 19, 2023

Conversation

blakewilson
Copy link
Contributor

@blakewilson blakewilson commented Dec 15, 2023

Tasks

  • I have signed a Contributor License Agreement (CLA) with WP Engine.
  • If a code change, I have written testing instructions that the whole team & outside contributors can understand.
  • I have written and included a comprehensive changeset to properly document the changes I've made.

Description

This PR creates a REST endpoint that accepts a body from the CLI, merges that data with data from WordPress, and then sends it off to GA if the user has Faust telemetry activated on WordPress.

The body looks like:

{
    "node_faustwp_core_version": "xxxx",
    "node_faustwp_cli_version": "xxxx",
    "node_faustwp_blocks_version": "xxxx",
    "node_apollo_client_version": "xxxx",
    "node_version": "xxxx",
    "node_next_version": "xxxx",
    "node_is_development": "xxxx",
    "command": "xxxx"
}

Related Issue(s):

Testing

  1. Checkout this branch
  2. Make a request to /wp-json/faustwp/v1/process_telemetry and see the 401 response since you didn't pass a secret key
  3. Add the secret key as the header x-faustwp-secret and verify you got a 204 response (no content).
  4. Modify the get_telemetry_client_id function to return a string and notice a 201 is returned (created).
  5. Try to break it

Screenshots

Documentation Changes

Dependant PRs

Copy link

changeset-bot bot commented Dec 15, 2023

⚠️ No Changeset found

Latest commit: b95940e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Comment on lines 110 to 120
function get_telemetry_client_id(): string|null {
/**
* @TODO Upon saving the site's telemetry decision, if they accept, we'll
* also need to generate a unique, anonymous client ID for them to be sent
* with GA requests.
*
* If a string is returned, telemetry is enabled and a client id has been generated.
* If this function returns null, either telemetry is off, or a client ID is not created.
*/
return null;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TeresaGobble When we save a user's preferences for their telemetry opt-in, we'll also want to generate a unique id for them (something like a UUID) as GA requires this. You can take a look at our previous implementation in the CLI for inspiration on how this can be done.

Copy link
Member

Choose a reason for hiding this comment

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

@blakewilson thoughts on using wp_generate_uuid4() for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mindctrl I think that's perfect!

Copy link
Contributor

github-actions bot commented Dec 15, 2023

📦 Next.js Bundle Analysis for @faustwp/getting-started-example

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@blakewilson blakewilson marked this pull request as ready for review December 18, 2023 18:15
@blakewilson blakewilson requested a review from a team as a code owner December 18, 2023 18:15
@blakewilson blakewilson changed the base branch from canary to telemetry-updates December 18, 2023 18:15
Copy link
Member

@theodesp theodesp left a comment

Choose a reason for hiding this comment

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

LGTM!

@blakewilson blakewilson merged commit cf383ab into telemetry-updates Dec 19, 2023
15 of 18 checks passed
@blakewilson blakewilson deleted the merl-1341-wp-telemetry branch December 19, 2023 15:43
mindctrl added a commit that referenced this pull request Jan 11, 2024
* Create process telemetry rest route (#1684)

* Create tests

* [Merl-1281] Add extra Google Analytics variables to Faust (#1689)

* Feat: Add block_editor_utils and app-router versions in telemetry events.

* Chore: Added changeset

* MERL-1343/MERL-1342: API rewrite to WP plugin vs GA (#1687)

* Removed telemetry from the CLI and moved it into the Faust WordPress plugin.

---------

Co-authored-by: Blake Wilson <blake.wilson@wpengine.com>
Co-authored-by: John Parris <john.parris@wpengine.com>

* MERL-1339: add WP notice for anonymous telemetry opt-in (#1690)

* Added checkbox for 'Enable Faust Telemetry'

* Added telemetry prompt to user page options

* Add `enable_telemetry` and `telemetry_reminder` to `sanitize_faustwp_settings()`

* Load telemetry callbacks file

* test: confirm `show_telemetry_prompt()` is hooked to `admin_notices`

* Sanitize telemetry reminder settings value by ensuring it's an integer

* Test telemetry prompt behavior

* test: confirm behavior of `should_show_telemetry_prompt()`

* fix: adjust CSS button styles to not style buttons in the telemetry prompt

* chore: adjust button classes and add aria-label attributes

* Register, enqueue, and localize telemetry script

* Register telemetry decision REST route

* test: confirm telemetry script is registered

* UX: Toggle checkbox on settings page to match user's decision.

Since this happens over REST via JS and we don't reload the page, this provides a better UX.

* test: confirm `should_show_telemetry_prompt()` returns false when telemetry disabled

* chore: add changeset

---------

Co-authored-by: John Parris <john.parris@wpengine.com>

* Generate UUID v4 for telemetry ID (#1693)

* chore: change test namespace from Unit to Integration

* chore: fix test class name to match file name

* fix: return 204 if telemetry not enabled

* test: confirm behavior of `generate_telemetry_client_id()`

* Support `telemetry_client_id` in settings sanitization function

* test: confirm generated and saved id matches retrieved id.


---------

Co-authored-by: Blake Wilson <blake.wilson@wpengine.com>

---------

Co-authored-by: Blake Wilson <blake.wilson@wpengine.com>
Co-authored-by: Theofanis Despoudis <328805+theodesp@users.noreply.github.com>
Co-authored-by: Matthew Wright <1815200+matthewguywright@users.noreply.github.com>
Co-authored-by: Teresa Gobble <teresagobble@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants