diff --git a/backend/src/database/repositories/githubReposRepository.ts b/backend/src/database/repositories/githubReposRepository.ts index 338c915197..594956967d 100644 --- a/backend/src/database/repositories/githubReposRepository.ts +++ b/backend/src/database/repositories/githubReposRepository.ts @@ -81,7 +81,7 @@ export default class GithubReposRepository { ) throw new Error400( options.language, - 'errors.integrations.githubRepoAlreadyMapped', + 'errors.integrations.repoAlreadyMapped', row.url, integrationId, row.integrationId, diff --git a/backend/src/database/repositories/gitlabReposRepository.ts b/backend/src/database/repositories/gitlabReposRepository.ts index a1c3066e33..227ec8cd20 100644 --- a/backend/src/database/repositories/gitlabReposRepository.ts +++ b/backend/src/database/repositories/gitlabReposRepository.ts @@ -1,7 +1,7 @@ import trim from 'lodash/trim' import { QueryTypes } from 'sequelize' -import { DEFAULT_TENANT_ID } from '@crowd/common' +import { DEFAULT_TENANT_ID, Error400 } from '@crowd/common' import { RedisCache } from '@crowd/redis' import { IRepositoryOptions } from './IRepositoryOptions' @@ -51,6 +51,35 @@ export default class GitlabReposRepository { } static async updateMapping(integrationId, mapping, options: IRepositoryOptions) { + const transaction = SequelizeRepository.getTransaction(options) + + // Check for repositories already mapped to other integrations + for (const url of Object.keys(mapping)) { + const existingRows = await options.database.sequelize.query( + `SELECT * FROM "gitlabRepos" WHERE url = :url AND "deletedAt" IS NULL`, + { + replacements: { url }, + type: QueryTypes.SELECT, + transaction, + }, + ) + + for (const row of existingRows as any[]) { + if (!row.deletedAt && row.integrationId !== integrationId) { + options.log.warn( + `Trying to update gitlab repo ${row.url} mapping with integrationId ${integrationId} but it is already mapped to integration ${row.integrationId}!`, + ) + + throw new Error400( + options.language, + 'errors.integrations.repoAlreadyMapped', + row.url, + integrationId, + row.integrationId, + ) + } + } + } await GitlabReposRepository.bulkInsert( 'gitlabRepos', ['tenantId', 'integrationId', 'segmentId', 'url'], diff --git a/backend/src/database/repositories/segmentRepository.ts b/backend/src/database/repositories/segmentRepository.ts index a8257a5dea..576c719c05 100644 --- a/backend/src/database/repositories/segmentRepository.ts +++ b/backend/src/database/repositories/segmentRepository.ts @@ -401,6 +401,35 @@ class SegmentRepository extends RepositoryBase< return this.findById(records[0].id) } + async findByName(name: string, level: SegmentLevel) { + const transaction = this.transaction + + let findByNameQuery = `SELECT * FROM segments WHERE name = :name AND "tenantId" = :tenantId` + + if (level === SegmentLevel.SUB_PROJECT) { + findByNameQuery += ` and "parentSlug" is not null and "grandparentSlug" is not null` + } else if (level === SegmentLevel.PROJECT) { + findByNameQuery += ` and "parentSlug" is not null and "grandparentSlug" is null` + } else if (level === SegmentLevel.PROJECT_GROUP) { + findByNameQuery += ` and "parentSlug" is null and "grandparentSlug" is null` + } + + const records = await this.options.database.sequelize.query(findByNameQuery, { + replacements: { + name, + tenantId: this.options.currentTenant.id, + }, + type: QueryTypes.SELECT, + transaction, + }) + + if (records.length === 0) { + return null + } + + return this.findById(records[0].id) + } + async findInIds(ids: string[]): Promise { if (ids.length === 0) { return [] diff --git a/backend/src/services/integrationService.ts b/backend/src/services/integrationService.ts index 6d74c2cb90..85d8343167 100644 --- a/backend/src/services/integrationService.ts +++ b/backend/src/services/integrationService.ts @@ -4,7 +4,7 @@ import { request } from '@octokit/request' import axios, { AxiosRequestConfig, AxiosResponse } from 'axios' import lodash from 'lodash' import moment from 'moment' -import { Transaction } from 'sequelize' +import { QueryTypes, Transaction } from 'sequelize' import { EDITION, Error400, Error404, Error542, encryptData } from '@crowd/common' import { CommonIntegrationService, getGithubInstallationToken } from '@crowd/common_services' @@ -1391,6 +1391,38 @@ export default class IntegrationService { options, ) + // Check for repositories already mapped to other integrations + const seq = SequelizeRepository.getSequelize({ ...(options || this.options), transaction }) + const urls = remotes.map((r) => r.url) + + const existingRows = await seq.query( + ` + SELECT url, "integrationId" FROM git.repositories + WHERE url IN (:urls) AND "deletedAt" IS NULL + `, + { + replacements: { urls }, + type: QueryTypes.SELECT, + transaction, + }, + ) + + for (const row of existingRows as any[]) { + if (row.integrationId !== integration.id) { + this.options.log.warn( + `Trying to update git repo ${row.url} mapping with integrationId ${integration.id} but it is already mapped to integration ${row.integrationId}!`, + ) + + throw new Error400( + (options || this.options).language, + 'errors.integrations.repoAlreadyMapped', + row.url, + integration.id, + row.integrationId, + ) + } + } + // upsert repositories to git.repositories in order to be processed by git-integration V2 const qx = SequelizeRepository.getQueryExecutor({ ...(options || this.options), diff --git a/backend/src/services/segmentService.ts b/backend/src/services/segmentService.ts index a62188a02b..c4678a8638 100644 --- a/backend/src/services/segmentService.ts +++ b/backend/src/services/segmentService.ts @@ -45,6 +45,11 @@ export default class SegmentService extends LoggerBase { try { const segmentRepository = new SegmentRepository({ ...this.options, transaction }) + // Validate name and slug uniqueness if being updated to prevent bypassing creation restrictions + if (data.name || data.slug) { + await this.validateUpdateDuplicates(id, segment, data, segmentRepository) + } + // make sure non-lf projects' slug are namespaced appropriately if (data.isLF === false) data.slug = validateNonLfSlug(data.slug) // do the update @@ -84,6 +89,14 @@ export default class SegmentService extends LoggerBase { const segmentRepository = new SegmentRepository({ ...this.options, transaction }) try { + // Check for conflicts with existing segments + await this.validateSegmentConflicts( + segmentRepository, + data.name, + data.slug, + SegmentLevel.PROJECT_GROUP, + data.isLF, + ) // create project group const projectGroup = await segmentRepository.create(data) @@ -147,11 +160,19 @@ export default class SegmentService extends LoggerBase { throw new Error(`Project group ${data.parentName} does not exist.`) } - if (parent.isLF !== data.isLF) - throw new Error400(this.options.language, `settings.segments.errors.isLfNotMatchingParent`) - if (data.isLF === false) data.slug = validateNonLfSlug(data.slug) - try { + // Check for conflicts with existing segments + await this.validateSegmentConflicts( + segmentRepository, + data.name, + data.slug, + SegmentLevel.PROJECT, + data.isLF, + ) + + if (parent.isLF !== data.isLF) + throw new Error400(this.options.language, `settings.segments.errors.isLfNotMatchingParent`) + if (data.isLF === false) data.slug = validateNonLfSlug(data.slug) // create project const project = await segmentRepository.create({ ...data, parentId: parent.id }) @@ -212,6 +233,15 @@ export default class SegmentService extends LoggerBase { data.slug = validateNonLfSlug(data.slug) } + // Check for conflicts with existing segments + await this.validateSegmentConflicts( + segmentRepository, + data.name, + data.slug, + SegmentLevel.SUB_PROJECT, + parent.isLF, + ) + const grandparent = await segmentRepository.findBySlug( data.grandparentSlug, SegmentLevel.PROJECT_GROUP, @@ -548,6 +578,181 @@ export default class SegmentService extends LoggerBase { this.setMembersCount(segments, level, membersCountPerSegment) } + /** + * Validates that a segment name and/or slug don't conflict with existing segments. + * This is a centralized validation function used for both creation and updates. + * + * @param segmentRepository - Repository instance for database operations + * @param name - Segment name to check for conflicts (optional) + * @param slug - Segment slug to check for conflicts (optional) + * @param segmentType - Type of segment being validated (PROJECT_GROUP, PROJECT, SUB_PROJECT) + * @param isLF - Whether this is a Linux Foundation segment (affects slug formatting) + * @param excludeId - Segment ID to exclude from conflict checking (used for updates) + * + * @throws Error400 with appropriate error message if conflicts are found + */ + private async validateSegmentConflicts( + segmentRepository: SegmentRepository, + name?: string, + slug?: string, + segmentType?: SegmentLevel, + isLF?: boolean, + excludeId?: string, + ): Promise { + // Validate slug conflicts if slug is provided + if (slug) { + // For projects and sub-projects, we need to check both LF and non-LF formats + // to prevent conflicts across both formats + if (segmentType === SegmentLevel.PROJECT || segmentType === SegmentLevel.SUB_PROJECT) { + const baseSlug = slug.startsWith('nonlf_') ? slug.substring(6) : slug + const nonLfSlug = `nonlf_${baseSlug}` + + // Check for conflicts with LF format (no prefix) + const existingLfBySlug = await segmentRepository.findBySlug(baseSlug, segmentType) + if (existingLfBySlug && (!excludeId || existingLfBySlug.id !== excludeId)) { + await this.throwSegmentConflictError(segmentRepository, existingLfBySlug, 'slug', slug) + } + + // Check for conflicts with non-LF format (nonlf_ prefix) + const existingNonLfBySlug = await segmentRepository.findBySlug(nonLfSlug, segmentType) + if (existingNonLfBySlug && (!excludeId || existingNonLfBySlug.id !== excludeId)) { + await this.throwSegmentConflictError(segmentRepository, existingNonLfBySlug, 'slug', slug) + } + } else { + // For project groups, just check the exact slug + const existingBySlug = await segmentRepository.findBySlug(slug, segmentType) + if (existingBySlug && (!excludeId || existingBySlug.id !== excludeId)) { + await this.throwSegmentConflictError(segmentRepository, existingBySlug, 'slug', slug) + } + } + } + + // Validate name conflicts if name is provided + if (name) { + const existingByName = await segmentRepository.findByName(name, segmentType) + + // If we found a conflicting segment and it's not the one we're updating + if (existingByName && (!excludeId || existingByName.id !== excludeId)) { + await this.throwSegmentConflictError(segmentRepository, existingByName, 'name', name) + } + } + } + + /** + * Throws an appropriate error message when a segment conflict is detected. + * This method dynamically generates error messages based on the existing conflicting segment, + * including the correct parent name from the database (not from the input data). + * + * @param segmentRepository - Repository instance for database operations + * @param existingSegment - The segment that already exists and conflicts + * @param conflictType - Whether the conflict is on 'name' or 'slug' + * @param conflictValue - The conflicting name or slug value + * + * @throws Error400 with localized error message and appropriate parameters + */ + private async throwSegmentConflictError( + segmentRepository: SegmentRepository, + existingSegment: SegmentData, + conflictType: 'name' | 'slug', + conflictValue: string, + ): Promise { + const existingSegmentType = SegmentService.getSegmentType(existingSegment) + + let errorKey: string + let parentName: string | undefined + + switch (existingSegmentType) { + case SegmentLevel.PROJECT_GROUP: { + // Project groups don't have parents, so no parent name needed + errorKey = + conflictType === 'slug' + ? 'settings.segments.errors.projectGroupSlugExists' + : 'settings.segments.errors.projectGroupNameExists' + break + } + + case SegmentLevel.PROJECT: { + errorKey = + conflictType === 'slug' + ? 'settings.segments.errors.projectSlugExists' + : 'settings.segments.errors.projectNameExists' + + // Fetch the actual parent (project group) name from the database + // This fixes the bug where we were using the wrong parent name + const projectParent = await segmentRepository.findById(existingSegment.parentId) + parentName = projectParent?.name + break + } + + case SegmentLevel.SUB_PROJECT: { + errorKey = + conflictType === 'slug' + ? 'settings.segments.errors.subprojectSlugExists' + : 'settings.segments.errors.subprojectNameExists' + + // Fetch the actual parent (project) name from the database + // This fixes the bug where we were using the wrong parent name + const subprojectParent = await segmentRepository.findById(existingSegment.parentId) + parentName = subprojectParent?.name + break + } + + default: + throw new Error(`Unknown segment type: ${existingSegmentType}`) + } + + // Throw error with appropriate parameters based on segment type + if (parentName) { + throw new Error400(this.options.language, errorKey, conflictValue, parentName) + } else { + throw new Error400(this.options.language, errorKey, conflictValue) + } + } + + /** + * Validates that segment updates don't create conflicts with existing segments. + * Only validates fields that are actually being changed to avoid unnecessary checks. + * + * @param segmentId - ID of the segment being updated (excluded from conflict checks) + * @param segment - The current segment data before update + * @param data - The update data containing potentially changed fields + * @param segmentRepository - Repository instance for database operations + * + * @throws Error400 if the update would create conflicts with existing segments + */ + private async validateUpdateDuplicates( + segmentId: string, + segment: SegmentData, + data: SegmentUpdateData, + segmentRepository: SegmentRepository, + ): Promise { + const segmentType = SegmentService.getSegmentType(segment) + + // Only validate fields that are actually being changed + await this.validateSegmentConflicts( + segmentRepository, + data.name !== segment.name ? data.name : undefined, + data.slug !== segment.slug ? data.slug : undefined, + segmentType, + data.isLF !== undefined ? data.isLF : segment.isLF, + segmentId, // Exclude the current segment from conflict checks + ) + } + + private static getSegmentType(segment: SegmentData): SegmentLevel { + // Fallback to parent/grandparent logic if type not available + if (!segment.parentSlug && !segment.grandparentSlug) { + return SegmentLevel.PROJECT_GROUP + } + if (segment.parentSlug && !segment.grandparentSlug) { + return SegmentLevel.PROJECT + } + if (segment.parentSlug && segment.grandparentSlug) { + return SegmentLevel.SUB_PROJECT + } + throw new Error('Unable to determine segment type') + } + static async refreshSegments(options: IRepositoryOptions) { const repo = new SegmentRepository(options) for (let i = 0; i < options.currentSegments.length; i++) { diff --git a/services/libs/common/src/i18n/en.ts b/services/libs/common/src/i18n/en.ts index 9f2f7c758d..b577238e0c 100644 --- a/services/libs/common/src/i18n/en.ts +++ b/services/libs/common/src/i18n/en.ts @@ -32,6 +32,12 @@ const en = { segments: { errors: { isLfNotMatchingParent: `isLF doesn't match the parent project`, + subprojectSlugExists: `Sub-project with slug "{0}" already exists in project {1}.`, + projectSlugExists: `Project with slug "{0}" already exists in project group {1}.`, + projectGroupSlugExists: `Project Group with slug "{0}" already exists.`, + subprojectNameExists: `Sub-project {0} already exists in project {1}.`, + projectNameExists: `Project {0} already exists in project group {1}.`, + projectGroupNameExists: `Project Group {0} already exists.`, }, }, }, @@ -129,8 +135,8 @@ const en = { }, integrations: { badEndpoint: 'Bad endpoint: {0}', - githubRepoAlreadyMapped: - 'Trying to update github repo {0} mapping with integrationId {1} but it is already mapped to integration {2}!', + repoAlreadyMapped: + 'Trying to update repo {0} mapping with integrationId {1} but it is already mapped to integration {2}!', }, sentiment: { mood: 'Invalid sentiment data. The {1} property must exist and be a number.',