Skip to content

feat: Ensure teams with conflicting slugs owned by the user are migrated(handled in backend, frontend already had this restriction) #25291

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Nov 22, 2025
Merged

feat: Ensure teams with conflicting slugs owned by the user are migrated(handled in backend, frontend already had this restriction) #25291

merged 6 commits into from
Nov 22, 2025

Conversation

@hariombalhara
Copy link
Member

@hariombalhara hariombalhara commented Nov 20, 2025 •
edited by devin-ai-integration bot
Loading

What does this PR do?

This PR adds backend validation to prevent slug conflicts when creating organizations. It ensures that if a user owns a team with the same slug as the organization they're creating, that team is automatically marked for migration.

Key Changes:

  1. Added findOwnedTeamsByUserId method to TeamRepository to query teams where user is OWNER or ADMIN
  2. Created buildTeamsAndInvites method in BaseOnboardingService that:
    • Automatically detects teams with same slug as organization
    • Marks conflicting teams for migration (isBeingMigrated: true)
    • Filters empty team names and invite emails
  3. Refactored listOwnedTeamsHandler to use the new repository method instead of direct Prisma queries
  4. Added comprehensive tests for slug conflict scenarios

This ensures backend validation even if frontend validation is bypassed, preventing slug conflicts during organization creation. All classes inheriting from BaseOnboardingService automatically get this validation.

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

Test Scenario 1: Automatic Migration Detection

  1. Create a team with slug "acme-corp" as an OWNER
  2. Attempt to create an organization with slug "acme-corp"
  3. Expected: The team should be automatically added to the migration list with isBeingMigrated: true

Test Scenario 2: Existing Team in List

  1. Create a team with slug "acme-corp" as an OWNER
  2. Include that team in the onboarding request with isBeingMigrated: false
  3. Expected: The team should be updated to have isBeingMigrated: true

Test Scenario 3: Non-Conflicting Slugs

  1. Create a team with slug "different-team" as an OWNER
  2. Create an organization with slug "acme-corp"
  3. Expected: The team should NOT be added to the migration list

Test Scenario 4: Member Role

  1. Create a team with slug "acme-corp" where user is only a MEMBER (not OWNER/ADMIN)
  2. Attempt to create an organization with slug "acme-corp"
  3. Expected: The team should NOT be migrated (user doesn't own it)

Environment Variables:

  • Standard Cal.com environment variables
  • No additional configuration needed

Human Review Checklist

Please pay special attention to:

  1. Query Logic Equivalence: Verify that TeamRepository.findOwnedTeamsByUserId returns the same results as the original Prisma query in listOwnedTeamsHandler (filters organizations, checks OWNER/ADMIN roles, requires accepted membership)

  2. Automatic Migration Behavior: Confirm that automatically adding teams to the migration list is the desired behavior and won't cause unexpected side effects in production

  3. Method Signature Change: The filterTeamsAndInvites method was changed to async buildTeamsAndInvites. Verify all callers have been updated (both BillingEnabledOrgOnboardingService and SelfHostedOnboardingService)

  4. Test Coverage: Review test scenarios, especially edge cases around:

    • Teams already marked for migration
    • Non-conflicting slugs
    • User role permissions (OWNER/ADMIN vs MEMBER)

  5. Repository Pattern: Verify that instantiating TeamRepository directly in BaseOnboardingService follows the codebase's dependency injection patterns

Link to Devin run: https://app.devin.ai/sessions/b90a924a3ffb4a0cb03cb0c79f5f98e0
Requested by: [email protected] (@hariombalhara)

@hariombalhara
feat: add backend validation for conflicting team slugs during org on…
33d43ed 
…boarding

- Added findOwnedTeamsByUserId method to TeamRepository
- Created buildTeamsAndInvites method in BaseOnboardingService that automatically:
  - Detects teams with same slug as organization
  - Marks conflicting teams for migration (isBeingMigrated: true)
  - Filters empty team names and invite emails
- Updated BillingEnabledOrgOnboardingService to use new method
- Updated SelfHostedOnboardingService to use new method
- Added comprehensive tests for slug conflict scenarios

This ensures backend validation even if frontend is bypassed, preventing
slug conflicts during organization creation. All inheriting classes
automatically get this validation without code changes.
@vercel
Copy link

vercel bot commented Nov 20, 2025 •
edited
Loading

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 21, 2025 8:47pm
cal-eu Ignored Ignored Nov 21, 2025 8:47pm

@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Nov 20, 2025
@devin-ai-integration devin-ai-integration bot changed the title refactor: use TeamRepository in listOwnedTeamsHandler feat: add backend validation for conflicting team slugs during org onboarding Nov 20, 2025
@hariombalhara
refactor: use TeamRepository in listOwnedTeamsHandler
7494654 
Refactored listOwnedTeamsHandler to use TeamRepository.findOwnedTeamsByUserId
instead of direct Prisma queries. This:
- Reduces code duplication
- Ensures consistency across the codebase
- Follows repository pattern
- Makes the handler more maintainable
@hariombalhara hariombalhara force-pushed the fix-conflicting-team-migration branch from 50980dd to 7494654 Compare November 20, 2025 09:04
@hariombalhara hariombalhara marked this pull request as ready for review November 20, 2025 09:04
@graphite-app graphite-app bot requested a review from a team November 20, 2025 09:05
@hariombalhara hariombalhara changed the title feat: add backend validation for conflicting team slugs during org onboarding feat: add backend validation for conflicting team slugs during org onboarding same as on client side Nov 20, 2025
@hariombalhara hariombalhara changed the title feat: add backend validation for conflicting team slugs during org onboarding same as on client side feat: Ensure teams with conflicting slugs owned by the user are migrated(handled in backend, frontend already had this restriction) Nov 20, 2025
hariombalhara
hariombalhara commented Nov 20, 2025
View reviewed changes
packages/features/ee/organizations/lib/service/onboarding/BaseOnboardingService.ts
];
}

protected async buildTeamsAndInvites(
Copy link
Member Author

@hariombalhara hariombalhara Nov 20, 2025

Choose a reason for hiding this comment

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

Renamed filterTeamsAndInvites -> buildTeamsAndInvites

cubic-dev-ai[bot]
cubic-dev-ai bot reviewed Nov 20, 2025
View reviewed changes
Copy link
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 6 files

devin-ai-integration bot and others added 2 commits November 20, 2025 13:45
@devin-ai-integration @hariombalhara
fix: update tests to use renamed buildTeamsAndInvites method
0c3d3c3 
- Renamed testFilterTeamsAndInvites to testBuildTeamsAndInvites
- Made test wrapper method async to match the async buildTeamsAndInvites
- Added orgSlug parameter to all test calls
- Updated all 9 test cases to use await with the new method signature
- Fixed lint warnings by using proper types instead of 'any'
- Imported OnboardingIntentResult and User types
- Used Pick<User> for mockUser type
- Removed all 'as any' type casts

Fixes test failures where filterTeamsAndInvites was renamed to buildTeamsAndInvites in the base service.

Co-Authored-By: [email protected] <[email protected]>
@devin-ai-integration @hariombalhara
fix: mock TeamRepository in tests to prevent database calls
ca97202 
Added vi.mock for TeamRepository to avoid database calls in unit tests.
The buildTeamsAndInvites method now calls ensureConflictingSlugTeamIsMigrated
which uses TeamRepository.findOwnedTeamsByUserId(). Mocking this prevents
Prisma errors in CI while keeping the tests focused on filtering logic.

The mock returns an empty array so no teams are found for migration,
allowing the tests to verify the filtering behavior without database access.

Co-Authored-By: [email protected] <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Nov 20, 2025 •
edited
Loading

E2E results are ready!

@devin-ai-integration @hariombalhara
refactor: improve readability of ensureConflictingSlugTeamIsMigrated
6098feb 
Refactored the conditional logic in ensureConflictingSlugTeamIsMigrated
for better readability while preserving exact behavior. Changed from
manual array manipulation to using .map() for updating team migration
status. This is a cosmetic change with no functional differences.

Co-Authored-By: [email protected] <[email protected]>
@devin-ai-integration
Copy link
Contributor

devin-ai-integration bot commented Nov 21, 2025

Refactored for readability while preserving exact behavior. The code now uses .map() to update the team migration status, making it more concise and easier to read.

Udit-takkar
Udit-takkar approved these changes Nov 21, 2025
View reviewed changes
Copy link
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

Looks good now

@Udit-takkar Udit-takkar enabled auto-merge (squash) November 21, 2025 20:47
@Udit-takkar Udit-takkar merged commit e29aa20 into main Nov 22, 2025
59 of 62 checks passed
@Udit-takkar Udit-takkar deleted the fix-conflicting-team-migration branch November 22, 2025 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cubic-dev-ai cubic-dev-ai[bot] cubic-dev-ai[bot] left review comments

@Udit-takkar Udit-takkar Udit-takkar approved these changes

@sean-brydon sean-brydon Awaiting requested review from sean-brydon

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@hariombalhara @Udit-takkar @keithwillcode