Skip to content

Wire activation-funnel analytics events#1189

Open
udil-cloudinary wants to merge 2 commits into
developfrom
feature/analytics-poc-wiring
Open

Wire activation-funnel analytics events#1189
udil-cloudinary wants to merge 2 commits into
developfrom
feature/analytics-poc-wiring

Conversation

@udil-cloudinary

Copy link
Copy Markdown

Phase 1 POC, PR #2 of 2 — wires the activation-funnel events on top of the analytics infrastructure from #1187. See the POC definition and implementation plan.

Approach

Server-side events emit directly through the Analytics component; client-side wizard events go through the JS track() bridge → the internal /events REST route (which enriches them with the server-side param envelope).

Server-side

  • plugin_activated (step 1) — install-type detection in Analytics::stash_activation() (called from Utils::install): fresh_install / reactivation / upgrade / downgrade, derived from the db_version install marker + version_compare. Stashed in a transient and emitted on the next admin load (so full mandatory params + session_id are present), then cleared. A new register_deactivation_hook persists _cloudinary_last_active so days_since_last_active can be computed on reactivation. Params: activation_type, previous_version, new_version, days_since_last_active.
  • connection_test_result (3d) — in rest_test_connection (status, error_type, attempt_number).
  • wizard_setup_submitted (5) — in rest_save_wizard (media_library, non_media, advanced, status).
  • first_sync_started (8, one-time) — in Push_Sync::process_assets (asset_count).
  • first_api_consumption (9, one-time) — via the existing cloudinary_uploaded_asset action; emitted on the first successful upload, then suppressed (api_endpoint, asset_type).

Client-side (wizard.js): wizard_started, wizard_signup_clicked, wizard_connect_viewed, credentials_entry_started, credentials_format_validated, connection_test_started, wizard_settings_viewed, wizard_setting_toggled, wizard_completed, wizard_dashboard_clicked.

Notes:

  • One-time markers (first_sync_started, first_api_consumption) are guarded by wp_option flags.
  • connection_test_result is server-side (richer error info); the client forwards attempt_number on the test request so the server event carries it.
  • track() is now lazy-init so call-sites are order-independent. Still fail-silent end-to-end; nothing emits in production until cloudinary_analytics_enabled is on (default true) and an event fires.

QA notes

  1. Fresh install: activate on a clean site → on the next admin page, plugin_activated with activation_type=fresh_install, previous_version absent.
  2. Reactivation: deactivate, wait, reactivate → activation_type=reactivation with days_since_last_active. Bump/lower .version to exercise upgrade/downgrade.
  3. Wizard: walk the setup wizard and confirm the client events fire in order (Network tab → cloudinary/v1/events), connection_test_result carries attempt_number, wizard_setup_submitted fires on save.
  4. First use: run a sync → first_sync_started once; first successful upload → first_api_consumption once (re-syncing does not re-fire either).
  5. Fail-safe: block the collector host → wp-admin stays fully usable; no console errors, no PHP fatals.
  6. npm run lint:js, npm run lint:php, npm run build all pass.

Test matrix: WP 5.6 / 6.9.x / 7.0, PHP 7.4 / 8.x, single + multisite, classic + block.

Builds on the analytics infrastructure (#1187) to emit the activation
funnel defined in the spec. Server-side events go through the Analytics
component; client-side wizard events go through the JS track() bridge.

Server-side:
- plugin_activated (step 1) with install-type detection
  (fresh_install / reactivation / upgrade / downgrade) from the
  db_version install marker + version compare, emitted on the next admin
  load via a transient. Persists _cloudinary_last_active on deactivation
  to derive days_since_last_active.
- connection_test_result (3d) in rest_test_connection.
- wizard_setup_submitted (5) in rest_save_wizard.
- first_sync_started (8, one-time) in Push_Sync::process_assets.
- first_api_consumption (9, one-time) via the cloudinary_uploaded_asset action.

Client-side (wizard.js -> /events bridge): wizard_started,
wizard_signup_clicked, wizard_connect_viewed, credentials_entry_started,
credentials_format_validated, connection_test_started,
wizard_settings_viewed, wizard_setting_toggled, wizard_completed,
wizard_dashboard_clicked.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@hezzy-cloudinary hezzy-cloudinary left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Activation-funnel analytics review. Most notes are inline.

Note: the two §2.3 boolean-serialization comments target php/class-analytics.php:306 (is_multisite() in the send body) and :399 (sanitize_text_field in the REST bridge). Both lines are unchanged code outside this PR's diff, so GitHub can't thread them there — they're anchored to the nearest changed line (L200) with a Re: pointer.

Comment thread php/class-analytics.php
'activation_funnel',
9,
array(
'api_endpoint' => 'upload',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Re php/class-analytics.php:306'is_multisite' => is_multisite() in the track() send body (anchored here; L306 is outside this PR's diff)

Spec §2.3 violation: is_multisite is a PHP bool placed in the wp_remote_post body array, so WP serializes false"" and true"1". Spec §2.3 requires real booleans.

Recommend JSON-encoding the body so this — plus format_valid, enabled, and the wizard_setup_submitted flags — serialize as true JSON booleans.

Comment thread php/class-analytics.php
'activation_funnel',
9,
array(
'api_endpoint' => 'upload',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Re php/class-analytics.php:399sanitize_text_field( (string) $value ) in the REST bridge (anchored here; L399 is outside this PR's diff)

Same §2.3 issue from the bridge: sanitize_text_field( (string) $value ) flattens JS true/false to "1"/"". Preserve booleans once the wire format is decided.

Comment thread php/class-analytics.php
'activation_funnel',
9,
array(
'api_endpoint' => 'upload',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing http_status (spec §3 step 9). Thread the response code from $result.

Also api_endpoint is hardcoded to 'upload', but the activation signal is first successful upload or explicit (§3/§6) — derive it from $result or confirm explicit can't be first.

Comment thread php/class-connect.php
if ( $analytics ) {
$success = 'connection_success' === $result['type'];
$analytics->track(
'connection_test_result',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

connection_test_result missing http_status (spec §3 step 3d, which requires status/error_type/http_status/attempt_number).

Also L165 reads $result['type'] unguarded, before track()'s try/catch — guard with isset() in case a transport-level failure returns no type.

Comment thread php/class-connect.php
$analytics = $this->plugin->get_component( 'analytics' );
if ( $analytics ) {
$analytics->track(
'wizard_setup_submitted',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

wizard_setup_submitted missing http_status (spec §3 step 5). Also status is hardcoded 'success' — confirm no failure branch should emit 'error'.

Comment thread php/sync/class-push-sync.php Outdated

// Activation funnel: first sync started (emitted once).
$analytics = $this->plugin->get_component( 'analytics' );
if ( $analytics && ! get_option( '_cloudinary_first_sync_emitted' ) ) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

first_sync_started missing trigger param (auto/manual; spec §3 step 8).

The flag is also global, not per-account — spec defines this as one-time per account, so a second cloud never re-emits. Same for _cloudinary_first_api_emitted. Key by cloud_name or clear on disconnect, or note the deviation.

Also confirm count( $ids ) is the full first sync, not one thread/batch.

Comment thread src/js/components/wizard.js Outdated
case 1:
this.hide( this.back );
this.unlockNext();
Analytics.track( 'wizard_started', {}, 'activation_funnel', 2 );

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

wizard_started missing entry_point (auto_redirect/menu; spec §3 step 2). Also fires on back-navigation to step 1 — guard if a single entry per session is intended.

this.debounceConnect = setTimeout( () => {
const valid = this.evaluateConnectionString( value );
Analytics.track(
'credentials_format_validated',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

credentials_format_validated missing invalid_reason (spec §3 step 3b). Load-bearing for the credential-friction focus; evaluateConnectionString() should expose the failure reason.

return;
}
Analytics.track(
'wizard_completed',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

wizard_completed missing time_to_complete_sec (spec §3 step 6; impl-plan §5 calls it out). Record a timestamp at wizard_started and diff here.

- Collector body is now JSON-encoded so booleans (is_multisite,
  format_valid, enabled, wizard flags) stay real JSON booleans per spec
  §2.3; the REST bridge preserves bool/int/float and only sanitizes strings.
- connection_test_result: thread the real http_status (surfaced from the
  WP_Error code in test_connection); guard $result['type'] with isset().
- wizard_setup_submitted: add http_status.
- first_api_consumption: add http_status; one-time guard now keyed by
  cloud_name (per-account, re-emits after an account switch).
- first_sync_started: one-time guard keyed by cloud_name; add trigger
  (auto/manual via wp_doing_cron).
- wizard_started: add entry_point + single-fire guard; persist start time.
- credentials_format_validated: add invalid_reason.
- wizard_completed: add time_to_complete_sec.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@udil-cloudinary

Copy link
Copy Markdown
Author

Thanks @hezzy-cloudinary — great review. Addressed in f1ca636:

§2.3 boolean serialization (both notes) — the collector body is now JSON-encoded (wp_json_encode + application/json), so is_multisite, format_valid, enabled, and the wizard_setup_submitted flags travel as real JSON booleans. The REST bridge no longer flattens them — it preserves bool/int/float and only sanitize_text_fields strings. (This assumes the collector parses a JSON body into params; flagging for backend confirmation since the deactivation precedent used a GET query string.)

connection_test_result http_status + unguarded typetest_connection() now surfaces the HTTP code into $result['http_status'] (the API::call() WP_Error code is the response code; 200 on success, 0 for local format failures with no network call). The event threads it, and type is read via isset().

wizard_setup_submitted http_status / status — added http_status: 200. There's no server-side failure branch (settings->save() returns void and the REST call is 200 on completion), so status stays success.

first_api_consumption — added http_status (200; the cloudinary_uploaded_asset action only carries the result body, not the raw code, and a non-error result implies 2xx). api_endpoint stays upload because that action fires only for uploads — explicit() doesn't trigger it, and the first sync consumes via upload, so it can't be first here. One-time guard is now keyed by cloud_name (re-emits after an account switch).

first_sync_started — guard is now per-account (cloud_name-keyed), same fix as above. Added trigger (auto/manual via wp_doing_cron()). On asset_count: the queue is threaded/batched before process_assets, so a single grand total isn't available at this hook — it's the first processed batch (commented in code). Happy to move the emit to the queue-build step in a follow-up if you'd prefer the true total.

wizard_started — added entry_point (auto_redirect/menu, heuristic on document.referrer) and a single-fire guard so back-navigation to step 1 doesn't re-emit. Also persists wizardStartedAt for the timing below.

credentials_format_validated — added invalid_reason (missing_scheme/missing_cloud_name/missing_secret/invalid_api_key/invalid_format) via a lightweight invalidReason() helper.

wizard_completed — added time_to_complete_sec, diffed from the persisted wizardStartedAt (survives reloads via the wizard's localStorage config).

Lint (php/js) and build pass. Two items I'd value your read on: the JSON-body assumption on the collector, and whether first_sync_started should move to the queue-build step for a true total asset_count.

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.

2 participants