packages/app-store/server.ts (1)

72-83: Verify the credential filtering logic for organization users.

The current implementation replaces the original { userId } filter with team-based filtering when the user has teams. This could cause personal user credentials to be excluded for organization members, potentially breaking existing functionality.

The past review comment correctly identified this issue: "When the user belongs to an organization this branch replaces the previous { userId } filter with { teamId: { in: [user.organizationId] } }, causing personal credentials owned by the user to be ignored."

Run this script to verify the credential filtering behavior:

#!/bin/bash
# Description: Verify that personal user credentials are still accessible when user has teams.
# Expected: Personal credentials should remain accessible alongside team credentials.

# Search for similar patterns in the codebase to understand intended behavior
ast-grep --pattern $'{ 
  OR: [
    {
      teamId: {
        in: $$$
      }
    },
    $$$
  ]
}'

# Check if there are other places where user credentials are fetched
rg -A 10 -B 5 "userId.*OR.*teamId" --type ts

packages/app-store/server.ts (2)

134-137: Improve readability of the credential mapping.

The destructuring assignment in the map function could be clearer by extracting the mapping logic to a separate variable or using more descriptive intermediate steps.

Apply this diff to improve readability:

-      for (const { teamName, credentialId } of app.credentials.map((credential) => ({
-        teamName: credential.team?.name,
-        credentialId: credential.id,
-      }))) {
+      const credentialMappings = app.credentials.map((credential) => ({
+        teamName: credential.team?.name,
+        credentialId: credential.id,
+      }));
+      
+      for (const { teamName, credentialId } of credentialMappings) {

---

147-150: Remove commented deduplication logic or provide explanation.

The commented deduplication code suggests this was intentionally disabled, but without context it's unclear if this is temporary or permanent. This could lead to duplicate options in the UI.

Either remove the commented code if it's no longer needed, or add a comment explaining why deduplication was disabled:

-          // const existingOption = apps[groupByCategory].find((o) => o.value === option.value);
-          // if (!existingOption) {
           apps[groupByCategory] = [...apps[groupByCategory], option];
-          // }

Or add an explanatory comment:

+          // Deduplication disabled to support multiple credentials per app
           // const existingOption = apps[groupByCategory].find((o) => o.value === option.value);
           // if (!existingOption) {
           apps[groupByCategory] = [...apps[groupByCategory], option];
           // }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

packages/features/bookings/lib/getAllCredentialsForUsersOnEvent/getAllCredentials.test.ts (1)

77-89: Repeat the DRY + mockResolvedValue refactor in all similar blocks

Apply the same setUserRepoProfile(...) usage to these blocks to remove duplication and ensure async-consistent stubbing.

Also applies to: 162-173, 239-250, 343-355, 666-678

packages/features/bookings/lib/getAllCredentialsForUsersOnEvent/getAllCredentials.test.ts (1)

22-34: DRY the UserRepository mocking and prefer mockResolvedValue for async semantics

You repeatedly stub enrichUserWithItsProfileSkipPlatformCheck and wire mockImplementation inline. This is verbose and slightly misleading since the repo call is async. Suggest centralizing with a helper and using mockResolvedValue.

• Benefit: less duplication, clearer intent, easier to tweak across all tests.

Apply this diff to replace the inline stub in this block:

-    const mockEnrichUserWithItsProfileSkipPlatformCheck = vi.fn().mockReturnValue({
-      profile: null,
-    });
-
-    const mockUserRepository = vi.mocked(UserRepository);
-    if (mockUserRepository && typeof mockUserRepository.mockImplementation === "function") {
-      mockUserRepository.mockImplementation(
-        () =>
-          ({
-            enrichUserWithItsProfileSkipPlatformCheck: mockEnrichUserWithItsProfileSkipPlatformCheck,
-          } as any)
-      );
-    }
+    setUserRepoProfile(null);

Add this helper near the top of the file (after the vi.mock(...)):

function setUserRepoProfile(profile: { organizationId?: number } | null) {
  const mockedCtor = vi.mocked(UserRepository);
  if (mockedCtor && typeof mockedCtor.mockImplementation === "function") {
    mockedCtor.mockImplementation(
      () =>
        ({
          enrichUserWithItsProfileSkipPlatformCheck: vi.fn().mockResolvedValue({ profile }),
        } as any)
    );
  }
}

Also consider adding test hygiene:

afterEach(() => {
  vi.clearAllMocks();
});

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.