ITADN

Three duplicate top-up gates have drifted apart; the isActiveSubscription rename left a live alias

#14862Openchristian-byrne 创建于 18 天前
C
christian-byrnecommented
Follow-up from the incident-95 local-credits thread. ## Problem / Goal **1. `useTopUpUrlLoader` has drifted out of sync with the gate it claims to mirror.** `useTopUpUrlLoader.ts:80-82`: ```ts const willOpenTopUpMode = billingContext.isActiveSubscription.value && !billingContext.isFreeTier.value ``` Its own comment at `:76-79` says this "is the negation of the paywall gate in showTopUpCreditsDialog (dialogService.ts); keep the two in sync." It is no longer the negation — `dialogService.ts:340` now reads `if (isCloud && (!canAccessSubscriptionFeatures.value || isFreeTier.value))`. The loader has no `isCloud` term. Effect: on a build where `isFreeTier` has latched, the deep link still opens the real dialog but **suppresses the `add_api_credit_button_clicked` telemetry** for it, so deep-link top-ups undercount. Telemetry-only today, but it is a third duplicate gate on the exact code path that produced the P0, and the comment actively misleads the next reader. **2. https://github.com/Comfy-Org/ComfyUI_frontend/pull/11464's rename did not finish.** That PR renamed `isActiveSubscription` to `canAccessSubscriptionFeatures` because the old name lies — off cloud it is forced unconditionally `true` (`useSubscription.ts:47-51`). But `useBillingContext.ts:146` keeps `const isActiveSubscription = canAccessSubscriptionFeatures` as an explicit backward-compatibility alias, re-exported at `:348` and typed at `types.ts:129`. The old name is still live in roughly ten production files: `useBillingContext.ts:154` (inside `canRunWorkflows` itself), `useTopUpUrlLoader.ts:81`, `useBillingBanner.ts:20,42,52,65,78`, `useTeamPlan.ts:7,16`, `useWorkspaceMenuItems.ts:23,68`, `useWorkspaceUI.ts:158,224,239`, plus storybook mocks. The rename was motivated by this exact bug — a misleading name led call sites to combine signals wrongly. Landing the new name while leaving the old one in place keeps the hazard and now presents two names for one value. ## Proposed Solution - Give `useTopUpUrlLoader` the same gate as `dialogService.showTopUpCreditsDialog`, or better, have both read one shared predicate so they cannot drift again. Delete the "keep the two in sync" comment once they cannot. - Migrate the remaining `isActiveSubscription` call sites to `canAccessSubscriptionFeatures` and remove the alias and its type entry. ## Acceptance Criteria - [ ] `grep -r 'isActiveSubscription' src/` returns no production hits. - [ ] The top-up gate exists in exactly one place; `useTopUpUrlLoader` and `dialogService` cannot disagree by construction. - [ ] A test asserts the deep-link path emits `add_api_credit_button_clicked` in the same cases where `showTopUpCreditsDialog` opens the dialog.
1 条评论