Comfy-Org / Comfy-Org/ComfyUI_frontend

Refactor: Move modal_opened telemetry emission into showPricingTable keyed off reason

Open
#13,103 1 comment 1 reaction 1 assignee Claimed by @christian-byrne View on GitHub
enhancement
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

## Summary

Centralize the `modal_opened` telemetry event emission inside `showPricingTable` (keyed off `reason`) rather than at individual call sites, and remove the existing ad-hoc emit to avoid double-counting.

## Background

Raised during review of PR #13001 (Pricing Table Deep Link feature). The loader `usePricingTableUrlLoader` (line 60) manually emits `trackSubscription('modal_opened', { reason: 'deep_link' })` before calling `subscriptionDialog.showPricingTable(...)`. The only other place that emits `modal_opened` today is `useSubscription.ts:241`. New call sites to `showPricingTable` risk silently dropping the metric if they forget to emit the event themselves.

## Required Changes

1. **`src/platform/cloud/subscription/composables/useSubscriptionDialog.ts`** — Inside `showPricingTable` (and/or the shared open helper), emit `trackSubscription('modal_opened', { reason })` automatically when a `reason` is supplied, so all callers get the telemetry for free.

2. **`src/platform/cloud/subscription/composables/useSubscription.ts` (line 241)** — Remove the existing manual `modal_opened` emit that would now be duplicated by the centralised emission in step 1.

3. **`src/platform/cloud/subscription/composables/usePricingTableUrlLoader.ts` (line 60)** — Remove the manual `useTelemetry()?.trackSubscription('modal_opened', { reason: 'deep_link' })` call, since `showPricingTable` will now handle it.

4. Audit any other call sites of `showPricingTable` or related dialog openers to ensure no double-emission occurs after the refactor.

5. Update or add unit tests for the centralised emission path.

## Acceptance Criteria

- `modal_opened` is emitted exactly once per pricing-table open, regardless of the call site.
- All existing tests continue to pass; new tests cover the centralised emit path.
- No call site manually emits `modal_opened` for the pricing table.

## References

- PR: https://github.com/Comfy-Org/ComfyUI_frontend/pull/13001
- Review comment: https://github.com/Comfy-Org/ComfyUI_frontend/pull/13001#discussion_r3444857734
- Requested by: @christian-byrne

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.