Skip to content

fix: do not limit users when self-hosting#225

Merged
KMKoushik merged 1 commit intomainfrom
km/2025-09-11-team-limit-self-hosted
Sep 10, 2025
Merged

fix: do not limit users when self-hosting#225
KMKoushik merged 1 commit intomainfrom
km/2025-09-11-team-limit-self-hosted

Conversation

@KMKoushik
Copy link
Copy Markdown
Member

@KMKoushik KMKoushik commented Sep 10, 2025

Summary by CodeRabbit

  • Refactor
    • Standardized how team member limits are validated during invite creation, ensuring consistent enforcement across the app.
    • Maintains current behavior and error messages when limits are reached; no changes to workflows or permissions.
    • Improves reliability and prepares the system for easier future updates to limit policies.
    • No user action required; this change is behind-the-scenes and does not affect existing teams or invitations.

@vercel
Copy link
Copy Markdown

vercel bot commented Sep 10, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
unsend-marketing Ready Ready Preview Comment Sep 10, 2025 7:45pm

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Sep 10, 2025

Walkthrough

The team invite member-limit check in apps/web/src/server/service/team-service.ts was refactored to delegate to LimitService. The previous logic that computed limits using PLAN_LIMITS and member counts was removed. In createTeamInvite, a single call to LimitService.checkTeamMemberLimit(teamId) now determines if the limit is reached and throws the same UnsendApiError when exceeded. Imports were updated to remove PLAN_LIMITS and add LimitService. No exported/public entity signatures were changed.

Possibly related PRs

  • block sending emails on limits #216: Also modifies team invite member-limit logic in team-service.ts, altering LimitService usage versus PLAN_LIMITS and introducing caching/notification adjustments.

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The current title implies that self-hosted deployments will no longer enforce user limits, yet the diff chiefly centralizes the team member limit logic by migrating from a static PLAN_LIMITS calculation to a call into LimitService, without explicitly exposing any self-hosting conditional in the code shown, making the title misleading relative to the actual changes. Consider renaming the PR to directly reflect the code change—such as “refactor: delegate team member limit check to LimitService”—or, if bypassing limits for self-hosting is indeed implemented here, ensure the title and summary explicitly reference that behavior.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch km/2025-09-11-team-limit-self-hosted

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying usesend with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7b3bd10
Status: ✅  Deploy successful!
Preview URL: https://323032aa.usesend.pages.dev
Branch Preview URL: https://km-2025-09-11-team-limit-sel.usesend.pages.dev

View logs

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/web/src/server/service/team-service.ts (1)

165-171: Re-validate team member limit in invite acceptance
In apps/web/src/server/api/routers/invitiation.ts (at the ctx.db.teamUser.create call around line 60), invoke LimitService.checkTeamMemberLimit(invite.teamId) and throw if isLimitReached before creating the teamUser to prevent a TOCTOU race.

🧹 Nitpick comments (1)
apps/web/src/server/service/team-service.ts (1)

9-10: Use ~/ alias for LimitService import to match apps/web conventions.

Follow the repo guideline for src imports in apps/web.

Apply:

-import { LimitService } from "./limit-service";
+import { LimitService } from "~/server/service/limit-service";
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd40de0 and 7b3bd10.

📒 Files selected for processing (1)
  • apps/web/src/server/service/team-service.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

Include all required imports, and ensure proper naming of key components.

Files:

  • apps/web/src/server/service/team-service.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use 2-space indentation in TypeScript code (enforced by Prettier)
Use semicolons in TypeScript code (enforced by Prettier)
Do not use dynamic imports

Files:

  • apps/web/src/server/service/team-service.ts
**/*.{ts,tsx,md}

📄 CodeRabbit inference engine (AGENTS.md)

Format code with Prettier 3 via pnpm format for TypeScript and Markdown files

Files:

  • apps/web/src/server/service/team-service.ts
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

In apps/web, use the / alias for src imports (e.g., import { x } from "/utils/x")

Files:

  • apps/web/src/server/service/team-service.ts
🧠 Learnings (1)
📚 Learning: 2025-09-10T12:33:42.627Z
Learnt from: KMKoushik
PR: usesend/useSend#224
File: apps/web/src/server/public-api/api/emails/get-email.ts:63-74
Timestamp: 2025-09-10T12:33:42.627Z
Learning: In the useSend project, Prisma's findUnique method works with composite where clauses including id, teamId, and optional domainId fields in apps/web/src/server/public-api/api/emails/get-email.ts

Applied to files:

  • apps/web/src/server/service/team-service.ts
🧬 Code graph analysis (1)
apps/web/src/server/service/team-service.ts (1)
apps/web/src/server/service/limit-service.ts (1)
  • LimitService (14-241)
🔇 Additional comments (1)
apps/web/src/server/service/team-service.ts (1)

165-166: LGTM: centralized limit check correctly skips self-hosted.

Delegating to LimitService honors the cloud-only gating and aligns with “do not limit users when self-hosting.”

@KMKoushik KMKoushik merged commit 0167d13 into main Sep 10, 2025
6 checks passed
@KMKoushik KMKoushik deleted the km/2025-09-11-team-limit-self-hosted branch September 10, 2025 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant