Skip to content

refactor: Unify OrganizationRepository and remove duplicate PrismaOrganizationRepository#24869

Merged
hariombalhara merged 9 commits intofix-apiv2-team-membership-additionfrom
devin/refactor-organization-repository-clean-1762169998
Nov 3, 2025
Merged

refactor: Unify OrganizationRepository and remove duplicate PrismaOrganizationRepository#24869
hariombalhara merged 9 commits intofix-apiv2-team-membership-additionfrom
devin/refactor-organization-repository-clean-1762169998

Conversation

@hariombalhara
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara commented Nov 3, 2025

What does this PR do?

This PR refactors the OrganizationRepository to use a dependency injection (DI) container pattern and simplifies the Prisma client architecture by removing the read/write client separation.

Key Changes:

  • Converted OrganizationRepository from static methods to instance-based DI pattern with getOrganizationRepository() container
  • Simplified base OrganizationRepository constructor to accept only { prismaClient: PrismaClient } instead of { prismaClient, prismaWriteClient? }
  • Replaced dual this.prismaRead/this.prismaWrite properties with single this.prismaClient property
  • Updated API v2 OrganizationsRepository to pass only dbWrite.prisma as the single Prisma client (all operations now use write client)
  • Fixed test mock setup in verify-email.test.ts to return mocked repository instance instead of scenario helper
  • Added mockReset to organizationMock.ts for proper mock cleanup between tests
  • Optimized repeated getOrganizationRepository() calls by storing in local variables

Link to Devin run: https://app.devin.ai/sessions/81e6321b59504d80a099dea7ae58fc8b
Requested by: @hariombalhara (hariom@cal.com)

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. N/A - internal refactoring only, no API changes
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

Prerequisites:

  • Ensure database is set up with test data including organizations

Test Scenarios:

  1. Apps/Web Organization Operations:

    • Create a new organization via /settings/organizations/new
    • Edit an existing organization via /settings/admin/organizations/[id]/edit
    • Verify organization teams listing works at /settings/organizations/teams/other
    • Test email verification flow with auto-accept domains
    • Verify availability page renders correctly with organization context
  2. API v2 Integration:

    • Test API v2 organization endpoints (GET, POST, PUT, DELETE)
    • Verify all operations work correctly using write client for both reads and writes
    • Check that no errors occur from missing read client
  3. Automated Tests:

    TZ=UTC yarn test OrganizationRepository.test.ts
    TZ=UTC yarn test verify-email.test.ts
  4. Type Checking:

    yarn type-check:ci --force

Human Review Checklist

Critical Items:

  • Verify all static method calls were converted: Search the codebase for OrganizationRepository. to ensure no static method calls remain (should all be getOrganizationRepository().)
  • API v2 write client usage: Confirm that using only the write client for all API v2 operations (including reads) is acceptable. Previously, reads used a read replica.
  • DI container correctness: Verify that OrganizationRepository.module.ts correctly passes prismaClient to the repository constructor
  • Mock setup: Review changes to organizationMock.ts and verify test isolation is maintained with mockReset
  • Apps/web client usage: Confirm apps/web is using the correct Prisma client from the DI container (not hardcoded to write client)

Testing:

  • Run full test suite: TZ=UTC yarn test
  • Manually test organization creation flow end-to-end
  • Manually test API v2 organization endpoints
  • Verify CI passes completely (especially integration tests)

Known Risks:

  • Large refactoring touching 31 files across apps/web, API v2, and shared packages - potential for missed call sites
  • Architectural change from read/write separation to single client may impact database load patterns
  • Mock infrastructure changes could cause subtle test issues
  • Type-safe but significant API surface change from static to instance methods

Files Changed:

  • Base repository: packages/features/ee/organizations/repositories/OrganizationRepository.ts
  • DI container: packages/features/ee/organizations/di/OrganizationRepository.{container,module}.ts
  • API v2: apps/api/v2/src/modules/organizations/index/organizations.repository.ts
  • Apps/web: 6 page components updated to use DI container
  • TRPC handlers: 4 handlers updated to use DI container
  • Tests: 2 test files updated with corrected mock setup
  • Platform libraries: Export updated to include both OrganizationRepository and getOrganizationRepository

devin-ai-integration bot and others added 4 commits November 3, 2025 11:42
- Add constructor accepting deps object with prismaClient
- Convert all static methods to instance methods
- Add getOrganizationAutoAcceptSettings method
- Create singleton instance export in repository barrel file
- Update API v2 OrganizationsRepository to extend from OrganizationRepository
- Update all call sites to use singleton instance
- Add platform-libraries organizations.ts export
- Fix mock imports to use repository barrel
- Fix unsafe optional chaining in next-auth-options.ts
- Fix any types in test files with proper type inference

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
- Update imports from direct OrganizationRepository file to barrel export
- This ensures mocks work correctly in tests
- Fixes 202 failing tests related to organizationRepository mock

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
- Convert organizationMock to partial mock that preserves real class
- Add proper prisma mocks to failing test files
- Remove old OrganizationRepository mocks from test files
- This fixes test failures related to mock interception

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
- Export mockedSingleton as organizationRepositoryMock from organizationMock
- Update delegationCredential.test.ts to import and use the exported mock
- This fixes 'vi.mocked(...).mockResolvedValue is not a function' errors

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Nov 3, 2025
API v2 should import shared features through @calcom/platform-libraries
instead of directly from @calcom/features to maintain proper architectural
boundaries and packaging/licensing separation.

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
@vercel
Copy link
Copy Markdown

vercel bot commented Nov 3, 2025

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

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Nov 3, 2025 1:29pm
cal-eu Ignored Ignored Nov 3, 2025 1:29pm

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

5 issues found across 29 files

Prompt for AI agents (all 5 issues)

Understand the root cause of the following 5 issues and fix them.


<file name="packages/platform/libraries/organizations.ts">

<violation number="1" location="packages/platform/libraries/organizations.ts:1">
This refactoring is incomplete. While many call sites have been updated, the duplicate `PrismaOrganizationRepository` was not deleted and is still referenced in the dependency injection container (`packages/features/ee/organizations/di/OrganizationRepository.module.ts`). This leaves unused, duplicate code and contradicts the primary goal of the PR.</violation>
</file>

<file name="packages/features/ee/organizations/__mocks__/organizationMock.ts">

<violation number="1" location="packages/features/ee/organizations/__mocks__/organizationMock.ts:17">
Replacing the deep mock reset with vi.clearAllMocks stops clearing mock implementations, so any scenario-configured behavior lingers into later tests and makes results dependent on execution order. Please restore a deep reset on the mocked singleton in beforeEach.</violation>
</file>

<file name="packages/features/delegation-credentials/repositories/DelegationCredentialRepository.test.ts">

<violation number="1" location="packages/features/delegation-credentials/repositories/DelegationCredentialRepository.test.ts:10">
This override wipes out the Prismock client that DelegationCredentialRepository relies on. Removing the manual vi.mock keeps the existing Prisma test double intact.</violation>

<violation number="2" location="packages/features/delegation-credentials/repositories/DelegationCredentialRepository.test.ts:94">
Spy on organizationRepository.findByMemberEmail before setting a mockResolvedValue; otherwise this call throws because the method isn’t a vi.fn.</violation>
</file>

<file name="packages/features/ee/organizations/repositories/OrganizationRepository.ts">

<violation number="1" location="packages/features/ee/organizations/repositories/OrganizationRepository.ts:235">
Rule violated: **Enforce Singular Naming for Single-Item Functions**

`findUniqueNonPlatformOrgsByMatchingAutoAcceptEmail` returns a single organization (`getParsedTeam(org)`), but the name still uses the plural “Orgs”. Please rename it to a singular form to comply with the singular-naming rule for single-item return values.</violation>
</file>

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

@@ -1,4 +1,3 @@
export { OrganizationRepository, organizationRepository } from "@calcom/features/ee/organizations/repositories";
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 3, 2025

Choose a reason for hiding this comment

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

This refactoring is incomplete. While many call sites have been updated, the duplicate PrismaOrganizationRepository was not deleted and is still referenced in the dependency injection container (packages/features/ee/organizations/di/OrganizationRepository.module.ts). This leaves unused, duplicate code and contradicts the primary goal of the PR.

Prompt for AI agents
Address the following comment on packages/platform/libraries/organizations.ts at line 1:

<comment>This refactoring is incomplete. While many call sites have been updated, the duplicate `PrismaOrganizationRepository` was not deleted and is still referenced in the dependency injection container (`packages/features/ee/organizations/di/OrganizationRepository.module.ts`). This leaves unused, duplicate code and contradicts the primary goal of the PR.</comment>

<file context>
@@ -1,4 +1,3 @@
+export { OrganizationRepository, organizationRepository } from &quot;@calcom/features/ee/organizations/repositories&quot;;
 export { OrganizationMembershipService } from &quot;@calcom/features/ee/organizations/lib/service/OrganizationMembershipService&quot;;
 export type { IOrganizationRepository } from &quot;@calcom/features/ee/organizations/lib/repository/IOrganizationRepository&quot;;
</file context>
Fix with Cubic

const organizationMock = mockDeep<OrganizationModule>();
const OrganizationRepository = organizationMock.OrganizationRepository;
beforeEach(() => {
vi.clearAllMocks();
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 3, 2025

Choose a reason for hiding this comment

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

Replacing the deep mock reset with vi.clearAllMocks stops clearing mock implementations, so any scenario-configured behavior lingers into later tests and makes results dependent on execution order. Please restore a deep reset on the mocked singleton in beforeEach.

Prompt for AI agents
Address the following comment on packages/features/ee/organizations/__mocks__/organizationMock.ts at line 17:

<comment>Replacing the deep mock reset with vi.clearAllMocks stops clearing mock implementations, so any scenario-configured behavior lingers into later tests and makes results dependent on execution order. Please restore a deep reset on the mocked singleton in beforeEach.</comment>

<file context>
@@ -1,23 +1,28 @@
-const organizationMock = mockDeep&lt;OrganizationModule&gt;();
-const OrganizationRepository = organizationMock.OrganizationRepository;
+beforeEach(() =&gt; {
+  vi.clearAllMocks();
+});
 
</file context>

✅ Addressed in 4d02ff0

const setupOrganizationMock = (returnValue: any) => {
vi.mocked(OrganizationRepository.findByMemberEmail).mockResolvedValue(returnValue);
const setupOrganizationMock = (returnValue: Awaited<ReturnType<typeof organizationRepository.findByMemberEmail>>) => {
vi.mocked(organizationRepository.findByMemberEmail).mockResolvedValue(returnValue);
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 3, 2025

Choose a reason for hiding this comment

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

Spy on organizationRepository.findByMemberEmail before setting a mockResolvedValue; otherwise this call throws because the method isn’t a vi.fn.

Prompt for AI agents
Address the following comment on packages/features/delegation-credentials/repositories/DelegationCredentialRepository.test.ts at line 94:

<comment>Spy on organizationRepository.findByMemberEmail before setting a mockResolvedValue; otherwise this call throws because the method isn’t a vi.fn.</comment>

<file context>
@@ -92,8 +90,8 @@ const createTestDelegationCredential = async (overrides = {}) =&gt; {
-const setupOrganizationMock = (returnValue: any) =&gt; {
-  vi.mocked(OrganizationRepository.findByMemberEmail).mockResolvedValue(returnValue);
+const setupOrganizationMock = (returnValue: Awaited&lt;ReturnType&lt;typeof organizationRepository.findByMemberEmail&gt;&gt;) =&gt; {
+  vi.mocked(organizationRepository.findByMemberEmail).mockResolvedValue(returnValue);
 };
 
</file context>
Suggested change
vi.mocked(organizationRepository.findByMemberEmail).mockResolvedValue(returnValue);
vi.spyOn(organizationRepository, "findByMemberEmail").mockResolvedValue(returnValue);

✅ Addressed in 7c7c2fb

OrganizationRepository: {
findByMemberEmail: vi.fn(),
},
vi.mock("@calcom/prisma", () => ({
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 3, 2025

Choose a reason for hiding this comment

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

This override wipes out the Prismock client that DelegationCredentialRepository relies on. Removing the manual vi.mock keeps the existing Prisma test double intact.

Prompt for AI agents
Address the following comment on packages/features/delegation-credentials/repositories/DelegationCredentialRepository.test.ts at line 10:

<comment>This override wipes out the Prismock client that DelegationCredentialRepository relies on. Removing the manual vi.mock keeps the existing Prisma test double intact.</comment>

<file context>
@@ -2,22 +2,20 @@ import prismock from &quot;../../../../tests/libs/__mocks__/prisma&quot;;
-  OrganizationRepository: {
-    findByMemberEmail: vi.fn(),
-  },
+vi.mock(&quot;@calcom/prisma&quot;, () =&gt; ({
+  prisma: {},
 }));
</file context>
Fix with Cubic

- Create OrganizationRepository.module.ts and .container.ts for DI
- Replace singleton pattern with getOrganizationRepository() across 20 files
- Update platform-libraries to export getOrganizationRepository
- Delete duplicate PrismaOrganizationRepository.ts
- Remove singleton export file (repositories/index.ts)
- Update test mocks to use DI container pattern
- All type checks and unit tests passing

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

Reviewed changes from recent commits (found 3 issues).

3 issues found across 28 files

Prompt for AI agents (all 3 issues)

Understand the root cause of the following 3 issues and fix them.


<file name="apps/web/app/(use-page-wrapper)/(main-nav)/availability/page.tsx">

<violation number="1" location="apps/web/app/(use-page-wrapper)/(main-nav)/availability/page.tsx:9">
The API v2 `OrganizationsRepository` initializes the base repository with a read-only Prisma client (`dbRead.prisma`). The base `OrganizationRepository` now contains write methods due to the refactoring, which will cause runtime errors when called from API v2 as they will attempt to write to a read-replica database.</violation>
</file>

<file name="apps/web/lib/pages/auth/verify-email.test.ts">

<violation number="1" location="apps/web/lib/pages/auth/verify-email.test.ts:14">
getOrganizationRepository should return the mocked repository instance, not the scenario helper object; otherwise findUniqueNonPlatformOrgsByMatchingAutoAcceptEmail is undefined and the test will throw.</violation>
</file>

<file name="apps/web/lib/pages/auth/verify-email.ts">

<violation number="1" location="apps/web/lib/pages/auth/verify-email.ts:24">
Rule violated: **Enforce Singular Naming for Single-Item Functions**

`findUniqueNonPlatformOrgsByMatchingAutoAcceptEmail` returns a single organization (it throws when multiple are found and returns `orgs[0]`), yet the method name still uses the plural `Orgs`, violating the “Enforce Singular Naming for Single-Item Functions” rule. Please rename the method to a singular form or adjust it to return a collection to match the naming.</violation>
</file>

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

Comment thread apps/web/app/(use-page-wrapper)/(main-nav)/availability/page.tsx
Comment thread apps/web/lib/pages/auth/verify-email.test.ts Outdated
Comment thread apps/web/lib/pages/auth/verify-email.ts
- Updated OrganizationRepository constructor to accept optional prismaWriteClient parameter
- Routed all write operations (create, update) through prismaWrite client
- Routed all read operations (find, get) through prismaRead client
- Updated API v2 OrganizationsRepository to pass both dbRead.prisma and dbWrite.prisma to super()
- Optimized getOrganizationRepository() calls by storing in local variables to avoid repeated function calls
- This fixes the critical issue where API v2 was passing read-only client to base class with write methods

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 4 files

devin-ai-integration bot and others added 2 commits November 3, 2025 13:17
- Fixed verify-email.test.ts mock to return mocked repository instance instead of scenario helper object
- Added mockReset to organizationMock.ts beforeEach to properly reset mock implementations between tests
- Added local variables in page.tsx to store getOrganizationRepository() result for consistency

This fixes the issue where getOrganizationRepository() was returning organizationScenarios.organizationRepository (scenario helper) instead of the actual mocked repository instance, causing findUniqueNonPlatformOrgsByMatchingAutoAcceptEmail to be undefined.

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
- Updated base OrganizationRepository constructor to accept only { prismaClient } instead of { prismaClient, prismaWriteClient? }
- Replaced this.prismaRead and this.prismaWrite with single this.prismaClient property
- Updated API v2 OrganizationsRepository to pass only dbWrite.prisma as prismaClient
- Removed unused PrismaReadService import from API v2
- All read and write operations now use the same client instance

This simplifies the architecture as requested - API v2 uses write client for all operations, and apps/web uses the client from DI container.

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 3 files

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

Reviewed changes from recent commits (found 1 issue).

1 issue found across 2 files

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="apps/api/v2/src/modules/organizations/index/organizations.repository.ts">

<violation number="1" location="apps/api/v2/src/modules/organizations/index/organizations.repository.ts:15">
This change directs all organization-related database operations, including reads, to the write database by instantiating the repository with the write client and switching local read calls to use dbWrite.</violation>
</file>

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

private readonly stripeService: StripeService
) {
super(dbRead.prisma);
super({ prismaClient: dbWrite.prisma });
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 3, 2025

Choose a reason for hiding this comment

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

This change directs all organization-related database operations, including reads, to the write database by instantiating the repository with the write client and switching local read calls to use dbWrite.

Prompt for AI agents
Address the following comment on apps/api/v2/src/modules/organizations/index/organizations.repository.ts at line 15:

<comment>This change directs all organization-related database operations, including reads, to the write database by instantiating the repository with the write client and switching local read calls to use dbWrite.</comment>

<file context>
@@ -10,15 +9,14 @@ import { Prisma } from &quot;@calcom/prisma/client&quot;;
     private readonly stripeService: StripeService
   ) {
-    super({ prismaClient: dbRead.prisma, prismaWriteClient: dbWrite.prisma });
+    super({ prismaClient: dbWrite.prisma });
   }
 
</file context>
Fix with Cubic

@hariombalhara hariombalhara marked this pull request as ready for review November 3, 2025 13:55
@hariombalhara hariombalhara requested a review from a team November 3, 2025 13:55
@hariombalhara hariombalhara requested review from a team as code owners November 3, 2025 13:55
@hariombalhara hariombalhara merged commit d848670 into fix-apiv2-team-membership-addition Nov 3, 2025
15 of 16 checks passed
@hariombalhara hariombalhara deleted the devin/refactor-organization-repository-clean-1762169998 branch November 3, 2025 13:55
@graphite-app graphite-app bot requested a review from a team November 3, 2025 13:55
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

5 issues found across 31 files

Prompt for AI agents (all 5 issues)

Understand the root cause of the following 5 issues and fix them.


<file name="apps/api/v2/src/modules/organizations/index/organizations.repository.ts">

<violation number="1" location="apps/api/v2/src/modules/organizations/index/organizations.repository.ts:19">
The query uses `include` which is against the project&#39;s data access guidelines. Please use `select` to specify the exact fields needed from the `team` and `platformBilling` tables to avoid over-fetching and improve performance and security.</violation>
</file>

<file name="packages/features/ee/organizations/repositories/OrganizationRepository.ts">

<violation number="1" location="packages/features/ee/organizations/repositories/OrganizationRepository.ts:204">
The query uses a top-level `include`, which fetches all columns from the `team` table and violates the project&#39;s data access guidelines. Please refactor the query to use `select` to specify only the required fields from the `team` table.</violation>

<violation number="2" location="packages/features/ee/organizations/repositories/OrganizationRepository.ts:235">
Rule violated: **Enforce Singular Naming for Single-Item Functions**

`findUniqueNonPlatformOrgsByMatchingAutoAcceptEmail` returns a single organization (or null), but the plural `Orgs` in its name violates the “Enforce Singular Naming for Single-Item Functions” rule. Rename the method to a singular form so callers understand it only ever yields one organization.</violation>
</file>

<file name="packages/features/ee/impersonation/lib/ImpersonationProvider.test.ts">

<violation number="1" location="packages/features/ee/impersonation/lib/ImpersonationProvider.test.ts:28">
This mock now only stubs a named export, but ImpersonationProvider pulls the default from `@calcom/prisma`, so the default resolves to `undefined` and any test exercising `prisma.*` will crash. Keep mocking the default export (or provide both) so prisma stays defined.</violation>
</file>

<file name="packages/features/delegation-credentials/repositories/DelegationCredentialRepository.test.ts">

<violation number="1" location="packages/features/delegation-credentials/repositories/DelegationCredentialRepository.test.ts:17">
Mocking `@calcom/prisma` to `{}` strips the repository of its Prisma client. DelegationCredentialRepository expects `prisma.delegationCredential.*`, so this change makes every call in the test throw instead of using prismock.</violation>
</file>

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.


async findById(organizationId: number) {
return this.dbRead.prisma.team.findUnique({
return this.dbWrite.prisma.team.findUnique({
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 3, 2025

Choose a reason for hiding this comment

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

The query uses include which is against the project's data access guidelines. Please use select to specify the exact fields needed from the team and platformBilling tables to avoid over-fetching and improve performance and security.

Prompt for AI agents
Address the following comment on apps/api/v2/src/modules/organizations/index/organizations.repository.ts at line 19:

<comment>The query uses `include` which is against the project&#39;s data access guidelines. Please use `select` to specify the exact fields needed from the `team` and `platformBilling` tables to avoid over-fetching and improve performance and security.</comment>

<file context>
@@ -1,24 +1,22 @@
 
   async findById(organizationId: number) {
-    return this.dbRead.prisma.team.findUnique({
+    return this.dbWrite.prisma.team.findUnique({
       where: {
         id: organizationId,
</file context>
Fix with Cubic


static async findBySlugIncludeOnboarding({ slug }: { slug: string }) {
return prisma.team.findFirst({
async findBySlugIncludeOnboarding({ slug }: { slug: string }) {
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 3, 2025

Choose a reason for hiding this comment

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

The query uses a top-level include, which fetches all columns from the team table and violates the project's data access guidelines. Please refactor the query to use select to specify only the required fields from the team table.

Prompt for AI agents
Address the following comment on packages/features/ee/organizations/repositories/OrganizationRepository.ts at line 204:

<comment>The query uses a top-level `include`, which fetches all columns from the `team` table and violates the project&#39;s data access guidelines. Please refactor the query to use `select` to specify only the required fields from the `team` table.</comment>

<file context>
@@ -176,27 +182,27 @@ export class OrganizationRepository {
 
-  static async findBySlugIncludeOnboarding({ slug }: { slug: string }) {
-    return prisma.team.findFirst({
+  async findBySlugIncludeOnboarding({ slug }: { slug: string }) {
+    return this.prismaClient.team.findFirst({
       where: { slug, isOrganization: true },
</file context>
Fix with Cubic

vi.mock("@calcom/prisma", () => {
return {
default: vi.fn(),
prisma: vi.fn(),
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 3, 2025

Choose a reason for hiding this comment

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

This mock now only stubs a named export, but ImpersonationProvider pulls the default from @calcom/prisma, so the default resolves to undefined and any test exercising prisma.* will crash. Keep mocking the default export (or provide both) so prisma stays defined.

Prompt for AI agents
Address the following comment on packages/features/ee/impersonation/lib/ImpersonationProvider.test.ts at line 28:

<comment>This mock now only stubs a named export, but ImpersonationProvider pulls the default from `@calcom/prisma`, so the default resolves to `undefined` and any test exercising `prisma.*` will crash. Keep mocking the default export (or provide both) so prisma stays defined.</comment>

<file context>
@@ -25,7 +25,7 @@ const session: Session = {
 vi.mock(&quot;@calcom/prisma&quot;, () =&gt; {
   return {
-    default: vi.fn(),
+    prisma: vi.fn(),
   };
 });
</file context>
Fix with Cubic

getOrganizationRepository: () => mockOrganizationRepository,
}));

vi.mock("@calcom/prisma", () => ({
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 3, 2025

Choose a reason for hiding this comment

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

Mocking @calcom/prisma to {} strips the repository of its Prisma client. DelegationCredentialRepository expects prisma.delegationCredential.*, so this change makes every call in the test throw instead of using prismock.

Prompt for AI agents
Address the following comment on packages/features/delegation-credentials/repositories/DelegationCredentialRepository.test.ts at line 17:

<comment>Mocking `@calcom/prisma` to `{}` strips the repository of its Prisma client. DelegationCredentialRepository expects `prisma.delegationCredential.*`, so this change makes every call in the test throw instead of using prismock.</comment>

<file context>
@@ -2,22 +2,27 @@ import prismock from &quot;../../../../tests/libs/__mocks__/prisma&quot;;
+  getOrganizationRepository: () =&gt; mockOrganizationRepository,
+}));
+
+vi.mock(&quot;@calcom/prisma&quot;, () =&gt; ({
+  prisma: {},
 }));
</file context>
Fix with Cubic

}

static async findUniqueNonPlatformOrgsByMatchingAutoAcceptEmail({ email }: { email: string }) {
async findUniqueNonPlatformOrgsByMatchingAutoAcceptEmail({ email }: { email: string }) {
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 3, 2025

Choose a reason for hiding this comment

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

Rule violated: Enforce Singular Naming for Single-Item Functions

findUniqueNonPlatformOrgsByMatchingAutoAcceptEmail returns a single organization (or null), but the plural Orgs in its name violates the “Enforce Singular Naming for Single-Item Functions” rule. Rename the method to a singular form so callers understand it only ever yields one organization.

Prompt for AI agents
Address the following comment on packages/features/ee/organizations/repositories/OrganizationRepository.ts at line 235:

<comment>`findUniqueNonPlatformOrgsByMatchingAutoAcceptEmail` returns a single organization (or null), but the plural `Orgs` in its name violates the “Enforce Singular Naming for Single-Item Functions” rule. Rename the method to a singular form so callers understand it only ever yields one organization.</comment>

<file context>
@@ -226,9 +232,9 @@ export class OrganizationRepository {
   }
 
-  static async findUniqueNonPlatformOrgsByMatchingAutoAcceptEmail({ email }: { email: string }) {
+  async findUniqueNonPlatformOrgsByMatchingAutoAcceptEmail({ email }: { email: string }) {
     const emailDomain = email.split(&quot;@&quot;).at(-1);
-    const orgs = await prisma.team.findMany({
</file context>
Fix with Cubic

hariombalhara added a commit that referenced this pull request Nov 7, 2025
…utomatically (#24780)

* fix: APIV2 team membership addition

* feat: Add trimming for email domain and orgAutoAcceptEmail in auto-accept logic

- Trim whitespace from both user email domain and orgAutoAcceptEmail
- Ensures consistent matching even with accidental whitespace
- Addresses feedback from PR review

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* simplify

* feat: Use shared OrganizationMembershipService in TRPC for consistent auto-accept logic

- Create OrganizationMembershipService.container.ts for DI in TRPC
- Update getOrgConnectionInfo to apply trimming + case-insensitive comparison
- Precompute auto-accept decisions in createNewUsersConnectToOrgIfExists using the service
- Use service in handleNewUsersInvites for consistent auto-accept determination
- Ensures both API v2 and TRPC paths use identical trimming and normalization logic

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* Revert "feat: Use shared OrganizationMembershipService in TRPC for consistent auto-accept logic"

This reverts commit 0b2bd28.

* refactor: Unify OrganizationRepository and remove duplicate PrismaOrganizationRepository (#24869)

* refactor: Convert OrganizationRepository from static to instance methods

- Add constructor accepting deps object with prismaClient
- Convert all static methods to instance methods
- Add getOrganizationAutoAcceptSettings method
- Create singleton instance export in repository barrel file
- Update API v2 OrganizationsRepository to extend from OrganizationRepository
- Update all call sites to use singleton instance
- Add platform-libraries organizations.ts export
- Fix mock imports to use repository barrel
- Fix unsafe optional chaining in next-auth-options.ts
- Fix any types in test files with proper type inference

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* fix: Update all imports to use OrganizationRepository barrel export

- Update imports from direct OrganizationRepository file to barrel export
- This ensures mocks work correctly in tests
- Fixes 202 failing tests related to organizationRepository mock

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* fix: Update test mocks to use partial mock pattern

- Convert organizationMock to partial mock that preserves real class
- Add proper prisma mocks to failing test files
- Remove old OrganizationRepository mocks from test files
- This fixes test failures related to mock interception

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* fix: Export mocked singleton and update tests to use it directly

- Export mockedSingleton as organizationRepositoryMock from organizationMock
- Update delegationCredential.test.ts to import and use the exported mock
- This fixes 'vi.mocked(...).mockResolvedValue is not a function' errors

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* fix: Use platform-libraries import for API v2 OrganizationRepository

API v2 should import shared features through @calcom/platform-libraries
instead of directly from @calcom/features to maintain proper architectural
boundaries and packaging/licensing separation.

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* refactor: Implement DI pattern for OrganizationRepository

- Create OrganizationRepository.module.ts and .container.ts for DI
- Replace singleton pattern with getOrganizationRepository() across 20 files
- Update platform-libraries to export getOrganizationRepository
- Delete duplicate PrismaOrganizationRepository.ts
- Remove singleton export file (repositories/index.ts)
- Update test mocks to use DI container pattern
- All type checks and unit tests passing

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* fix: Implement read/write client separation in OrganizationRepository

- Updated OrganizationRepository constructor to accept optional prismaWriteClient parameter
- Routed all write operations (create, update) through prismaWrite client
- Routed all read operations (find, get) through prismaRead client
- Updated API v2 OrganizationsRepository to pass both dbRead.prisma and dbWrite.prisma to super()
- Optimized getOrganizationRepository() calls by storing in local variables to avoid repeated function calls
- This fixes the critical issue where API v2 was passing read-only client to base class with write methods

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* fix: Fix mock setup and optimize getOrganizationRepository() calls

- Fixed verify-email.test.ts mock to return mocked repository instance instead of scenario helper object
- Added mockReset to organizationMock.ts beforeEach to properly reset mock implementations between tests
- Added local variables in page.tsx to store getOrganizationRepository() result for consistency

This fixes the issue where getOrganizationRepository() was returning organizationScenarios.organizationRepository (scenario helper) instead of the actual mocked repository instance, causing findUniqueNonPlatformOrgsByMatchingAutoAcceptEmail to be undefined.

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* refactor: Simplify OrganizationRepository to use single prismaClient

- Updated base OrganizationRepository constructor to accept only { prismaClient } instead of { prismaClient, prismaWriteClient? }
- Replaced this.prismaRead and this.prismaWrite with single this.prismaClient property
- Updated API v2 OrganizationsRepository to pass only dbWrite.prisma as prismaClient
- Removed unused PrismaReadService import from API v2
- All read and write operations now use the same client instance

This simplifies the architecture as requested - API v2 uses write client for all operations, and apps/web uses the client from DI container.

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix: Match OrganizationsRepository.findById signature with base class

The findById method in OrganizationsRepository was using a different signature
than the base OrganizationRepository class, causing type errors in CI.

Changed from: findById(organizationId: number)
Changed to: findById({ id }: { id: number })

This matches the base class signature and resolves the CI unit test failures.

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* fix: Update all findById call sites to use object parameter

Fixed 6 call sites in API v2 that were calling findById with a number
instead of the required { id: number } object parameter:

- is-org.guard.ts
- is-admin-api-enabled.guard.ts
- is-webhook-in-org.guard.ts
- organizations.service.ts
- managed-organizations.service.ts (2 call sites)

This resolves the API v2 build failure caused by the signature change
in OrganizationsRepository.findById to match the base class.

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* fix: Remove redundant findById override from OrganizationsRepository

The findById method was duplicating the base class OrganizationRepository
implementation. Both methods had identical logic (filtering by isOrganization: true),
so the override was unnecessary.

Since OrganizationsRepository extends OrganizationRepository and passes
dbWrite.prisma to the base constructor, the base class method already
provides the exact same functionality.

This resolves the API v2 build failure by eliminating the duplicate method
that was causing conflicts.

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* Remove unncessary changes

* Store in variable

* Revert "Remove unncessary changes"

This reverts commit af93517.

* Revert dbRead/dbWrite changes

* Add organizations library to tsconfig.json

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Morgan <33722304+ThyMinimalDev@users.noreply.github.com>
akarsh-jain-790 pushed a commit to akarsh-jain-790/cal.com that referenced this pull request Mar 8, 2026
…utomatically (calcom#24780)

* fix: APIV2 team membership addition

* feat: Add trimming for email domain and orgAutoAcceptEmail in auto-accept logic

- Trim whitespace from both user email domain and orgAutoAcceptEmail
- Ensures consistent matching even with accidental whitespace
- Addresses feedback from PR review

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* simplify

* feat: Use shared OrganizationMembershipService in TRPC for consistent auto-accept logic

- Create OrganizationMembershipService.container.ts for DI in TRPC
- Update getOrgConnectionInfo to apply trimming + case-insensitive comparison
- Precompute auto-accept decisions in createNewUsersConnectToOrgIfExists using the service
- Use service in handleNewUsersInvites for consistent auto-accept determination
- Ensures both API v2 and TRPC paths use identical trimming and normalization logic

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* Revert "feat: Use shared OrganizationMembershipService in TRPC for consistent auto-accept logic"

This reverts commit 0b2bd28.

* refactor: Unify OrganizationRepository and remove duplicate PrismaOrganizationRepository (calcom#24869)

* refactor: Convert OrganizationRepository from static to instance methods

- Add constructor accepting deps object with prismaClient
- Convert all static methods to instance methods
- Add getOrganizationAutoAcceptSettings method
- Create singleton instance export in repository barrel file
- Update API v2 OrganizationsRepository to extend from OrganizationRepository
- Update all call sites to use singleton instance
- Add platform-libraries organizations.ts export
- Fix mock imports to use repository barrel
- Fix unsafe optional chaining in next-auth-options.ts
- Fix any types in test files with proper type inference

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* fix: Update all imports to use OrganizationRepository barrel export

- Update imports from direct OrganizationRepository file to barrel export
- This ensures mocks work correctly in tests
- Fixes 202 failing tests related to organizationRepository mock

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* fix: Update test mocks to use partial mock pattern

- Convert organizationMock to partial mock that preserves real class
- Add proper prisma mocks to failing test files
- Remove old OrganizationRepository mocks from test files
- This fixes test failures related to mock interception

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* fix: Export mocked singleton and update tests to use it directly

- Export mockedSingleton as organizationRepositoryMock from organizationMock
- Update delegationCredential.test.ts to import and use the exported mock
- This fixes 'vi.mocked(...).mockResolvedValue is not a function' errors

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* fix: Use platform-libraries import for API v2 OrganizationRepository

API v2 should import shared features through @calcom/platform-libraries
instead of directly from @calcom/features to maintain proper architectural
boundaries and packaging/licensing separation.

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* refactor: Implement DI pattern for OrganizationRepository

- Create OrganizationRepository.module.ts and .container.ts for DI
- Replace singleton pattern with getOrganizationRepository() across 20 files
- Update platform-libraries to export getOrganizationRepository
- Delete duplicate PrismaOrganizationRepository.ts
- Remove singleton export file (repositories/index.ts)
- Update test mocks to use DI container pattern
- All type checks and unit tests passing

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* fix: Implement read/write client separation in OrganizationRepository

- Updated OrganizationRepository constructor to accept optional prismaWriteClient parameter
- Routed all write operations (create, update) through prismaWrite client
- Routed all read operations (find, get) through prismaRead client
- Updated API v2 OrganizationsRepository to pass both dbRead.prisma and dbWrite.prisma to super()
- Optimized getOrganizationRepository() calls by storing in local variables to avoid repeated function calls
- This fixes the critical issue where API v2 was passing read-only client to base class with write methods

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* fix: Fix mock setup and optimize getOrganizationRepository() calls

- Fixed verify-email.test.ts mock to return mocked repository instance instead of scenario helper object
- Added mockReset to organizationMock.ts beforeEach to properly reset mock implementations between tests
- Added local variables in page.tsx to store getOrganizationRepository() result for consistency

This fixes the issue where getOrganizationRepository() was returning organizationScenarios.organizationRepository (scenario helper) instead of the actual mocked repository instance, causing findUniqueNonPlatformOrgsByMatchingAutoAcceptEmail to be undefined.

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* refactor: Simplify OrganizationRepository to use single prismaClient

- Updated base OrganizationRepository constructor to accept only { prismaClient } instead of { prismaClient, prismaWriteClient? }
- Replaced this.prismaRead and this.prismaWrite with single this.prismaClient property
- Updated API v2 OrganizationsRepository to pass only dbWrite.prisma as prismaClient
- Removed unused PrismaReadService import from API v2
- All read and write operations now use the same client instance

This simplifies the architecture as requested - API v2 uses write client for all operations, and apps/web uses the client from DI container.

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix: Match OrganizationsRepository.findById signature with base class

The findById method in OrganizationsRepository was using a different signature
than the base OrganizationRepository class, causing type errors in CI.

Changed from: findById(organizationId: number)
Changed to: findById({ id }: { id: number })

This matches the base class signature and resolves the CI unit test failures.

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* fix: Update all findById call sites to use object parameter

Fixed 6 call sites in API v2 that were calling findById with a number
instead of the required { id: number } object parameter:

- is-org.guard.ts
- is-admin-api-enabled.guard.ts
- is-webhook-in-org.guard.ts
- organizations.service.ts
- managed-organizations.service.ts (2 call sites)

This resolves the API v2 build failure caused by the signature change
in OrganizationsRepository.findById to match the base class.

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* fix: Remove redundant findById override from OrganizationsRepository

The findById method was duplicating the base class OrganizationRepository
implementation. Both methods had identical logic (filtering by isOrganization: true),
so the override was unnecessary.

Since OrganizationsRepository extends OrganizationRepository and passes
dbWrite.prisma to the base constructor, the base class method already
provides the exact same functionality.

This resolves the API v2 build failure by eliminating the duplicate method
that was causing conflicts.

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>

* Remove unncessary changes

* Store in variable

* Revert "Remove unncessary changes"

This reverts commit af93517.

* Revert dbRead/dbWrite changes

* Add organizations library to tsconfig.json

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Morgan <33722304+ThyMinimalDev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants