-
Notifications
You must be signed in to change notification settings - Fork 673
[backport cloud/1.38] fix: route gtm through telemetry entrypoint #8714
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| name: 'CI: Dist Telemetry Scan' | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches-ignore: [wip/*, draft/*, temp/*] | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| scan: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 | ||
|
|
||
| - name: Install pnpm | ||
| uses: pnpm/action-setup@41ff72655975bd51cab0327fa583b6e92b6d3061 # v4.2.0 | ||
| with: | ||
| version: 10 | ||
|
|
||
| - name: Use Node.js | ||
| uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 | ||
| with: | ||
| node-version: 'lts/*' | ||
| cache: 'pnpm' | ||
|
|
||
| - name: Install dependencies | ||
| run: pnpm install --frozen-lockfile | ||
|
|
||
| - name: Build project | ||
| run: pnpm build | ||
|
|
||
| - name: Scan dist for telemetry references | ||
| run: | | ||
| set -euo pipefail | ||
| if rg --no-ignore -n \ | ||
| -g '*.html' \ | ||
| -g '*.js' \ | ||
| -e 'Google Tag Manager' \ | ||
| -e '(?i)\bgtm\.js\b' \ | ||
| -e '(?i)googletagmanager\.com/gtm\.js\\?id=' \ | ||
| -e '(?i)googletagmanager\.com/ns\.html\\?id=' \ | ||
| dist; then | ||
| echo 'Telemetry references found in dist assets.' | ||
| exit 1 | ||
| fi | ||
| echo 'No telemetry references found in dist assets.' | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -14,15 +14,18 @@ const mockSubscriptionTier = ref< | |||||
| const mockIsYearlySubscription = ref(false) | ||||||
| const mockAccessBillingPortal = vi.fn() | ||||||
| const mockReportError = vi.fn() | ||||||
| const mockTrackBeginCheckout = vi.fn() | ||||||
| const mockGetFirebaseAuthHeader = vi.fn(() => | ||||||
| Promise.resolve({ Authorization: 'Bearer test-token' }) | ||||||
| ) | ||||||
| const mockGetCheckoutAttribution = vi.hoisted(() => vi.fn(() => ({}))) | ||||||
|
|
||||||
| vi.mock('@/platform/cloud/subscription/composables/useSubscription', () => ({ | ||||||
| useSubscription: () => ({ | ||||||
| isActiveSubscription: computed(() => mockIsActiveSubscription.value), | ||||||
| subscriptionTier: computed(() => mockSubscriptionTier.value), | ||||||
| isYearlySubscription: computed(() => mockIsYearlySubscription.value) | ||||||
| isYearlySubscription: computed(() => mockIsYearlySubscription.value), | ||||||
| subscriptionStatus: ref(null) | ||||||
| }) | ||||||
| })) | ||||||
|
|
||||||
|
|
@@ -53,11 +56,22 @@ vi.mock('@/composables/useErrorHandling', () => ({ | |||||
|
|
||||||
| vi.mock('@/stores/firebaseAuthStore', () => ({ | ||||||
| useFirebaseAuthStore: () => ({ | ||||||
| getFirebaseAuthHeader: mockGetFirebaseAuthHeader | ||||||
| getFirebaseAuthHeader: mockGetFirebaseAuthHeader, | ||||||
| userId: 'user-123' | ||||||
|
||||||
| userId: 'user-123' | |
| userId: computed(() => 'user-123') |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -266,6 +266,9 @@ import { performSubscriptionCheckout } from '@/platform/cloud/subscription/utils | |||||||||||||
| import { isPlanDowngrade } from '@/platform/cloud/subscription/utils/subscriptionTierRank' | ||||||||||||||
| import type { BillingCycle } from '@/platform/cloud/subscription/utils/subscriptionTierRank' | ||||||||||||||
| import { isCloud } from '@/platform/distribution/types' | ||||||||||||||
| import { useTelemetry } from '@/platform/telemetry' | ||||||||||||||
| import { getCheckoutAttribution } from '@/platform/telemetry/utils/checkoutAttribution' | ||||||||||||||
| import { useFirebaseAuthStore } from '@/stores/firebaseAuthStore' | ||||||||||||||
| import type { components } from '@/types/comfyRegistryTypes' | ||||||||||||||
|
|
||||||||||||||
| type SubscriptionTier = components['schemas']['SubscriptionTier'] | ||||||||||||||
|
|
@@ -330,6 +333,8 @@ const tiers: PricingTierConfig[] = [ | |||||||||||||
| const { n } = useI18n() | ||||||||||||||
| const { isActiveSubscription, subscriptionTier, isYearlySubscription } = | ||||||||||||||
| useSubscription() | ||||||||||||||
| const telemetry = useTelemetry() | ||||||||||||||
| const { userId } = useFirebaseAuthStore() | ||||||||||||||
| const { accessBillingPortal, reportError } = useFirebaseAuthActions() | ||||||||||||||
| const { wrapWithErrorHandlingAsync } = useErrorHandling() | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -410,6 +415,19 @@ const handleSubscribe = wrapWithErrorHandlingAsync( | |||||||||||||
|
|
||||||||||||||
| try { | ||||||||||||||
| if (isActiveSubscription.value) { | ||||||||||||||
| const checkoutAttribution = getCheckoutAttribution() | ||||||||||||||
| if (userId) { | ||||||||||||||
| telemetry?.trackBeginCheckout({ | ||||||||||||||
| user_id: userId, | ||||||||||||||
|
Comment on lines
+419
to
+421
|
||||||||||||||
| if (userId) { | |
| telemetry?.trackBeginCheckout({ | |
| user_id: userId, | |
| if (userId?.value) { | |
| telemetry?.trackBeginCheckout({ | |
| user_id: userId.value, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new scan patterns use
\?id=(googletagmanager\.com/...\\?id=), which in ripgrep regex does not match a literal?id=URL query; as a result, GTM URL references like.../ns.html?id=...can slip past CI undetected. This weakens the guardrail the workflow is intended to enforce for telemetry-free dist assets.Useful? React with 👍 / 👎.