-
Notifications
You must be signed in to change notification settings - Fork 516
Feat/workspaces 6 billing #8508
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
781548d
b0618a6
03535f7
54f7044
d3792da
620cb17
d16396c
26c1766
f01d6b3
18d90d7
fd58e8f
6398070
a7da719
7e81c80
9e962ac
4d9316e
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 |
|---|---|---|
|
|
@@ -96,3 +96,5 @@ vitest.config.*.timestamp* | |
|
|
||
| # Weekly docs check output | ||
| /output.txt | ||
|
|
||
| .amp | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,295 @@ | ||
| <template> | ||
| <div | ||
| class="flex min-w-[460px] flex-col rounded-2xl border border-border-default bg-base-background shadow-[1px_1px_8px_0_rgba(0,0,0,0.4)]" | ||
| > | ||
| <!-- Header --> | ||
| <div class="flex py-8 items-center justify-between px-8"> | ||
| <h2 class="text-lg font-bold text-base-foreground m-0"> | ||
| {{ | ||
| isInsufficientCredits | ||
| ? $t('credits.topUp.addMoreCreditsToRun') | ||
| : $t('credits.topUp.addMoreCredits') | ||
| }} | ||
| </h2> | ||
| <button | ||
| class="cursor-pointer rounded border-none bg-transparent p-0 text-muted-foreground transition-colors hover:text-base-foreground focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-secondary-foreground" | ||
| @click="() => handleClose()" | ||
| > | ||
| <i class="icon-[lucide--x] size-6" /> | ||
| </button> | ||
|
Comment on lines
+14
to
+19
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use the shared IconButton and add an accessible label for the close control. Based on learnings: In Vue files across the ComfyUI_frontend repo, when a button is needed, prefer the repo's common button components from src/components/button/ (IconButton.vue, TextButton.vue, IconTextButton.vue) over plain HTML elements; and for icon-only buttons, provide a clear aria-label or aria-labelledby. 🤖 Prompt for AI Agents |
||
| </div> | ||
| <p | ||
| v-if="isInsufficientCredits" | ||
| class="text-sm text-muted-foreground m-0 px-8" | ||
| > | ||
| {{ $t('credits.topUp.insufficientWorkflowMessage') }} | ||
| </p> | ||
|
|
||
| <!-- Preset amount buttons --> | ||
| <div class="px-8"> | ||
| <h3 class="m-0 text-sm font-normal text-muted-foreground"> | ||
| {{ $t('credits.topUp.selectAmount') }} | ||
| </h3> | ||
| <div class="flex gap-2 pt-3"> | ||
| <Button | ||
| v-for="amount in PRESET_AMOUNTS" | ||
| :key="amount" | ||
| :autofocus="amount === 50" | ||
| variant="secondary" | ||
| size="lg" | ||
| :class=" | ||
| cn( | ||
| 'h-10 text-base font-medium w-full focus-visible:ring-secondary-foreground', | ||
| selectedPreset === amount && 'bg-secondary-background-selected' | ||
| ) | ||
| " | ||
| @click="handlePresetClick(amount)" | ||
| > | ||
| ${{ amount }} | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| <!-- Amount (USD) / Credits --> | ||
| <div class="flex gap-2 px-8 pt-8"> | ||
| <!-- You Pay --> | ||
| <div class="flex flex-1 flex-col gap-3"> | ||
| <div class="text-sm text-muted-foreground"> | ||
| {{ $t('credits.topUp.youPay') }} | ||
| </div> | ||
| <FormattedNumberStepper | ||
| :model-value="payAmount" | ||
| :min="0" | ||
| :max="MAX_AMOUNT" | ||
| :step="getStepAmount" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: it seems like it should be a computed prop rather than a getter. |
||
| @update:model-value="handlePayAmountChange" | ||
| @max-reached="showCeilingWarning = true" | ||
| > | ||
| <template #prefix> | ||
| <span class="shrink-0 text-base font-semibold text-base-foreground" | ||
| >$</span | ||
| > | ||
| </template> | ||
| </FormattedNumberStepper> | ||
| </div> | ||
|
|
||
| <!-- You Get --> | ||
| <div class="flex flex-1 flex-col gap-3"> | ||
| <div class="text-sm text-muted-foreground"> | ||
| {{ $t('credits.topUp.youGet') }} | ||
| </div> | ||
| <FormattedNumberStepper | ||
| v-model="creditsModel" | ||
| :min="0" | ||
| :max="usdToCredits(MAX_AMOUNT)" | ||
| :step="getCreditsStepAmount" | ||
| @max-reached="showCeilingWarning = true" | ||
| > | ||
| <template #prefix> | ||
| <i class="icon-[lucide--component] size-4 shrink-0 text-gold-500" /> | ||
| </template> | ||
| </FormattedNumberStepper> | ||
| </div> | ||
| </div> | ||
|
|
||
| <!-- Warnings --> | ||
|
|
||
| <p | ||
| v-if="isBelowMin" | ||
| class="text-sm text-red-500 m-0 px-8 pt-4 text-center flex items-center justify-center gap-1" | ||
| > | ||
| <i class="icon-[lucide--component] size-4" /> | ||
| {{ | ||
| $t('credits.topUp.minRequired', { | ||
| credits: formatNumber(usdToCredits(MIN_AMOUNT)) | ||
| }) | ||
| }} | ||
| </p> | ||
| <p | ||
| v-if="showCeilingWarning" | ||
| class="text-sm text-gold-500 m-0 px-8 pt-4 text-center flex items-center justify-center gap-1" | ||
| > | ||
| <i class="icon-[lucide--component] size-4" /> | ||
| {{ | ||
| $t('credits.topUp.maxAllowed', { | ||
| credits: formatNumber(usdToCredits(MAX_AMOUNT)) | ||
| }) | ||
| }} | ||
| <span>{{ $t('credits.topUp.needMore') }}</span> | ||
| <a | ||
| href="https://www.comfy.org/cloud/enterprise" | ||
| target="_blank" | ||
| class="ml-1 text-inherit" | ||
| >{{ $t('credits.topUp.contactUs') }}</a | ||
| > | ||
|
Comment on lines
+118
to
+123
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add 🔒 Safer link attributes <a
href="https://www.comfy.org/cloud/enterprise"
target="_blank"
+ rel="noopener noreferrer"
class="ml-1 text-inherit"
>{{ $t('credits.topUp.contactUs') }}</a
>
@@
<a
:href="pricingUrl"
target="_blank"
+ rel="noopener noreferrer"
class="flex items-center gap-1 text-sm text-muted-foreground no-underline transition-colors hover:text-base-foreground"
>Also applies to: 138-145 🤖 Prompt for AI Agents |
||
| </p> | ||
|
|
||
| <div class="pt-8 pb-8 flex flex-col gap-8 px-8"> | ||
| <Button | ||
| :disabled="!isValidAmount || loading || isPolling" | ||
| :loading="loading || isPolling" | ||
| variant="primary" | ||
| size="lg" | ||
| class="h-10 justify-center" | ||
| @click="handleBuy" | ||
| > | ||
| {{ $t('subscription.addCredits') }} | ||
| </Button> | ||
| <div class="flex items-center justify-center gap-1"> | ||
| <a | ||
| :href="pricingUrl" | ||
| target="_blank" | ||
| class="flex items-center gap-1 text-sm text-muted-foreground no-underline transition-colors hover:text-base-foreground" | ||
| > | ||
| {{ $t('credits.topUp.viewPricing') }} | ||
| <i class="icon-[lucide--external-link] size-4" /> | ||
| </a> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </template> | ||
|
|
||
| <script setup lang="ts"> | ||
| import { useToast } from 'primevue/usetoast' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check PrimeVue dependency presence/version and import usages.
rg -n '"primevue"' package.json pnpm-lock.yaml
rg -n "primevue/usetoast" -g'*.{ts,vue}'Repository: Comfy-Org/ComfyUI_frontend Length of output: 3367 🏁 Script executed: #!/bin/bash
# Look for toast usage patterns and alternatives
rg -n "Toast|toast" src/components/toast -A 3 -B 1
rg -n "import.*[Tt]oast" --stats | head -20
# Check if there's a custom toast composable
fd -e ts -e vue src | xargs grep -l "GlobalToast\|useToast" | head -10Repository: Comfy-Org/ComfyUI_frontend Length of output: 11113 🏁 Script executed: #!/bin/bash
# Check the usage of useToast in the flagged file
cat -n src/components/dialog/content/TopUpCreditsDialogContentWorkspace.vue | grep -A 10 -B 5 "useToast"Repository: Comfy-Org/ComfyUI_frontend Length of output: 1437 Replace PrimeVue PrimeVue usage should be avoided in new Vue components. Use 🧰 Tools🪛 ESLint[error] 152-152: Unable to resolve path to module 'primevue/usetoast'. (import-x/no-unresolved) 🤖 Prompt for AI Agents |
||
| import { computed, ref } from 'vue' | ||
| import { useI18n } from 'vue-i18n' | ||
|
|
||
| import { creditsToUsd, usdToCredits } from '@/base/credits/comfyCredits' | ||
| import Button from '@/components/ui/button/Button.vue' | ||
| import FormattedNumberStepper from '@/components/ui/stepper/FormattedNumberStepper.vue' | ||
| import { useBillingContext } from '@/composables/billing/useBillingContext' | ||
| import { useExternalLink } from '@/composables/useExternalLink' | ||
| import { useTelemetry } from '@/platform/telemetry' | ||
| import { clearTopupTracking } from '@/platform/telemetry/topupTracker' | ||
| import { workspaceApi } from '@/platform/workspace/api/workspaceApi' | ||
| import { useDialogService } from '@/services/dialogService' | ||
| import { useBillingOperationStore } from '@/stores/billingOperationStore' | ||
| import { useDialogStore } from '@/stores/dialogStore' | ||
| import { cn } from '@/utils/tailwindUtil' | ||
|
|
||
| const { isInsufficientCredits = false } = defineProps<{ | ||
| isInsufficientCredits?: boolean | ||
| }>() | ||
|
|
||
| const { t } = useI18n() | ||
| const dialogStore = useDialogStore() | ||
| const dialogService = useDialogService() | ||
| const telemetry = useTelemetry() | ||
| const toast = useToast() | ||
| const { buildDocsUrl, docsPaths } = useExternalLink() | ||
| const { fetchBalance } = useBillingContext() | ||
|
|
||
| const billingOperationStore = useBillingOperationStore() | ||
| const isPolling = computed(() => billingOperationStore.hasPendingOperations) | ||
|
|
||
| // Constants | ||
| const PRESET_AMOUNTS = [10, 25, 50, 100] | ||
| const MIN_AMOUNT = 5 | ||
| const MAX_AMOUNT = 10000 | ||
|
|
||
| // State | ||
| const selectedPreset = ref<number | null>(50) | ||
| const payAmount = ref(50) | ||
| const showCeilingWarning = ref(false) | ||
| const loading = ref(false) | ||
|
|
||
| // Computed | ||
| const pricingUrl = computed(() => | ||
| buildDocsUrl(docsPaths.partnerNodesPricing, { includeLocale: true }) | ||
| ) | ||
|
|
||
| const creditsModel = computed({ | ||
| get: () => usdToCredits(payAmount.value), | ||
| set: (newCredits: number) => { | ||
| payAmount.value = Math.round(creditsToUsd(newCredits)) | ||
| selectedPreset.value = null | ||
| } | ||
|
Comment on lines
+200
to
+205
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clear the ceiling warning when the credits stepper updates.
🛠️ Suggested fix const creditsModel = computed({
get: () => usdToCredits(payAmount.value),
set: (newCredits: number) => {
payAmount.value = Math.round(creditsToUsd(newCredits))
selectedPreset.value = null
+ showCeilingWarning.value = false
}
})🤖 Prompt for AI Agents |
||
| }) | ||
|
|
||
| const isValidAmount = computed( | ||
| () => payAmount.value >= MIN_AMOUNT && payAmount.value <= MAX_AMOUNT | ||
| ) | ||
|
|
||
| const isBelowMin = computed(() => payAmount.value < MIN_AMOUNT) | ||
|
|
||
| // Utility functions | ||
| function formatNumber(num: number): string { | ||
| return num.toLocaleString('en-US') | ||
| } | ||
|
Comment on lines
+215
to
+217
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The 💡 Suggested fix-function formatNumber(num: number): string {
- return num.toLocaleString('en-US')
-}Then in the template, replace As per coding guidelines: Use 🤖 Prompt for AI Agents |
||
|
|
||
| // Step amount functions | ||
| function getStepAmount(currentAmount: number): number { | ||
| if (currentAmount < 100) return 5 | ||
| if (currentAmount < 1000) return 50 | ||
| return 100 | ||
| } | ||
|
|
||
| function getCreditsStepAmount(currentCredits: number): number { | ||
| const usdAmount = creditsToUsd(currentCredits) | ||
| return usdToCredits(getStepAmount(usdAmount)) | ||
| } | ||
|
|
||
| // Event handlers | ||
| function handlePayAmountChange(value: number) { | ||
| payAmount.value = value | ||
| selectedPreset.value = null | ||
| showCeilingWarning.value = false | ||
| } | ||
|
|
||
| function handlePresetClick(amount: number) { | ||
| showCeilingWarning.value = false | ||
| payAmount.value = amount | ||
| selectedPreset.value = amount | ||
| } | ||
|
|
||
| function handleClose(clearTracking = true) { | ||
| if (clearTracking) { | ||
| clearTopupTracking() | ||
| } | ||
| dialogStore.closeDialog({ key: 'top-up-credits' }) | ||
| } | ||
|
|
||
| async function handleBuy() { | ||
| if (loading.value || !isValidAmount.value) return | ||
|
|
||
| loading.value = true | ||
| try { | ||
| telemetry?.trackApiCreditTopupButtonPurchaseClicked(payAmount.value) | ||
|
|
||
| const amountCents = payAmount.value * 100 | ||
| const response = await workspaceApi.createTopup(amountCents) | ||
|
Comment on lines
+258
to
+259
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Normalize cents to an integer before calling the API. If the stepper allows fractional USD values, 🧮 Suggested fix- const amountCents = payAmount.value * 100
+ const amountCents = Math.round(payAmount.value * 100)🤖 Prompt for AI Agents |
||
|
|
||
| if (response.status === 'completed') { | ||
| toast.add({ | ||
| severity: 'success', | ||
| summary: t('credits.topUp.purchaseSuccess'), | ||
| life: 5000 | ||
| }) | ||
| await fetchBalance() | ||
| handleClose(false) | ||
| dialogService.showSettingsDialog('workspace') | ||
|
Comment on lines
+268
to
+269
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we worried at all about the timing if this has possibility of tearing down the state of the current component? |
||
| } else if (response.status === 'pending') { | ||
| billingOperationStore.startOperation(response.billing_op_id, 'topup') | ||
| } else { | ||
| toast.add({ | ||
| severity: 'error', | ||
| summary: t('credits.topUp.purchaseError'), | ||
| detail: t('credits.topUp.unknownError'), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WDYT about sending something to Sentry here and below -- to benefit our future selves if we have to debug. |
||
| life: 5000 | ||
| }) | ||
| } | ||
| } catch (error) { | ||
| console.error('Purchase failed:', error) | ||
|
|
||
| const errorMessage = | ||
| error instanceof Error ? error.message : t('credits.topUp.unknownError') | ||
| toast.add({ | ||
| severity: 'error', | ||
| summary: t('credits.topUp.purchaseError'), | ||
| detail: t('credits.topUp.purchaseErrorDetail', { error: errorMessage }), | ||
| life: 5000 | ||
| }) | ||
| } finally { | ||
| loading.value = false | ||
| } | ||
| } | ||
| </script> | ||
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.
It feels like for organization, it would be nicer to put this (and other files) in a
src/platform/workspacefolder rather than src/components. Then it might be easy to see all the files together and handle the transition better.