Chore(webapp): Adds additional billing alerts #2829
Conversation
|
WalkthroughThe pull request introduces two changes: a syntax fix in Tooltip.tsx that adds a trailing comma in variantClasses, and a new optional Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.billing-alerts/route.tsx (1)
278-310: LGTM! Spike alerts section well-implemented.The new Spike alerts UI group is well-structured with:
- Clear labeling and helpful tooltip explaining the purpose
- Proper use of
formatNumberfor large percentages (displays as 1,000%, 2,000%, etc.)- Consistent formatting with the existing alert levels section
- Correct form field binding to
alertLevels.namefor unified submissionThe
defaultCheckedlogic correctly implements the intended behavior: preserve existing settings from the database, but enable all spike alerts by default for organizations that don't have them set yet (both new orgs and existing orgs seeing this feature for the first time).💡 Optional: Add explanatory comment for defaultChecked logic
The
defaultCheckedlogic at lines 303-306 is correct but relatively complex. Consider adding a brief comment to clarify the intent for future maintainers:defaultChecked={ + // Preserve DB settings if spike alert exists, otherwise default to checked (safety net for new/existing orgs) alerts.alertLevels.includes(level) || !spikeAlertLevels.some((l) => alerts.alertLevels.includes(l)) }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/components/primitives/Tooltip.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.billing-alerts/route.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.billing-alerts/route.tsxapps/webapp/app/components/primitives/Tooltip.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.billing-alerts/route.tsxapps/webapp/app/components/primitives/Tooltip.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.billing-alerts/route.tsxapps/webapp/app/components/primitives/Tooltip.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.billing-alerts/route.tsxapps/webapp/app/components/primitives/Tooltip.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.billing-alerts/route.tsxapps/webapp/app/components/primitives/Tooltip.tsx
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.billing-alerts/route.tsxapps/webapp/app/components/primitives/Tooltip.tsx
🧬 Code graph analysis (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.billing-alerts/route.tsx (2)
apps/webapp/app/components/primitives/Tooltip.tsx (1)
InfoIconTooltip(109-133)apps/webapp/app/utils/numberFormatter.ts (2)
formatNumber(9-11)formatCurrency(25-27)
apps/webapp/app/components/primitives/Tooltip.tsx (1)
packages/core/src/v3/schemas/style.ts (1)
Variant(8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/webapp/app/components/primitives/Tooltip.tsx (2)
9-9: LGTM! Formatting consistency improved.The trailing comma follows Prettier conventions and maintains consistency with multi-line object formatting.
61-61: LGTM! New prop properly implemented.The
disableHoverableContentprop is correctly threaded through the component hierarchy fromInfoIconTooltip→SimpleTooltip→TooltipProvider. The default value offalsemaintains backward compatibility, and the prop is properly typed throughout.Also applies to: 75-75, 85-85, 114-114, 120-120, 130-130
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.billing-alerts/route.tsx (2)
27-27: LGTM! Appropriate imports added.The imports for
InfoIconTooltipandformatNumberare correctly added to support the new spike alerts UI functionality.Also applies to: 33-33
187-187: LGTM! Spike alert thresholds well-defined.The spike alert levels (10x, 20x, 50x, 100x) are appropriate for catching runaway usage scenarios and align with the PR's objective of providing a "safety net" for customers.
Updated the billing alerts documentation to reflect the new spike alerts feature added in PR #2829. The documentation now explains both standard alerts (75%, 90%, 100%, 200%, 500%) and spike alerts (10x, 20x, 50x, 100x) that help catch runaway usage from bugs or errors. ## Files changed - `docs/how-to-reduce-your-spend.mdx` - Added section explaining the two types of billing alerts Generated from [Chore(webapp): Adds additional billing alerts](#2829) @samejr Co-authored-by: mintlify[bot] <109931778+mintlify[bot]@users.noreply.github.com>
defaultCheckedlogic to honour existing orgs who have configured alerts in the DB. New orgs get all alerts checked on by default.