From 44f340da5718029905a2edd1940bfaf3506d944c Mon Sep 17 00:00:00 2001 From: Joana Maia Date: Tue, 16 Dec 2025 18:22:12 +0000 Subject: [PATCH 1/8] chore: improve api error messages --- .../repositories/githubReposRepository.ts | 2 +- .../repositories/gitlabReposRepository.ts | 31 ++++++++- .../repositories/segmentRepository.ts | 29 +++++++++ backend/src/services/segmentService.ts | 64 +++++++++++++++++++ services/libs/common/src/i18n/en.ts | 10 ++- 5 files changed, 132 insertions(+), 4 deletions(-) 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..fdce06eb75 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/segmentService.ts b/backend/src/services/segmentService.ts index a62188a02b..cc43532ecc 100644 --- a/backend/src/services/segmentService.ts +++ b/backend/src/services/segmentService.ts @@ -83,6 +83,26 @@ export default class SegmentService extends LoggerBase { const collectionService = new CollectionService({ ...this.options, transaction }) const segmentRepository = new SegmentRepository({ ...this.options, transaction }) + // Check for existing project group with same slug + const existingBySlug = await segmentRepository.findBySlug(data.slug, SegmentLevel.PROJECT_GROUP) + if (existingBySlug) { + throw new Error400( + this.options.language, + 'settings.segments.errors.projectGroupSlugExists', + data.slug, + ) + } + + // Check for existing project group with same name + const existingByName = await segmentRepository.findByName(data.name, SegmentLevel.PROJECT_GROUP) + if (existingByName) { + throw new Error400( + this.options.language, + 'settings.segments.errors.projectGroupNameExists', + data.name, + ) + } + try { // create project group const projectGroup = await segmentRepository.create(data) @@ -147,6 +167,28 @@ export default class SegmentService extends LoggerBase { throw new Error(`Project group ${data.parentName} does not exist.`) } + // Check for existing project with same slug in the project group + const existingBySlug = await segmentRepository.findBySlug(data.slug, SegmentLevel.PROJECT) + if (existingBySlug && existingBySlug.parentSlug === data.parentSlug) { + throw new Error400( + this.options.language, + 'settings.segments.errors.projectSlugExists', + data.slug, + parent.name, + ) + } + + // Check for existing project with same name in the project group + const existingByName = await segmentRepository.findByName(data.name, SegmentLevel.PROJECT) + if (existingByName && existingByName.parentSlug === data.parentSlug) { + throw new Error400( + this.options.language, + 'settings.segments.errors.projectNameExists', + data.name, + parent.name, + ) + } + if (parent.isLF !== data.isLF) throw new Error400(this.options.language, `settings.segments.errors.isLfNotMatchingParent`) if (data.isLF === false) data.slug = validateNonLfSlug(data.slug) @@ -212,6 +254,28 @@ export default class SegmentService extends LoggerBase { data.slug = validateNonLfSlug(data.slug) } + // Check for existing subproject with same slug in the project + const existingBySlug = await segmentRepository.findBySlug(data.slug, SegmentLevel.SUB_PROJECT) + if (existingBySlug && existingBySlug.parentSlug === data.parentSlug) { + throw new Error400( + this.options.language, + 'settings.segments.errors.subprojectSlugExists', + data.slug, + parent.name, + ) + } + + // Check for existing subproject with same name in the project + const existingByName = await segmentRepository.findByName(data.name, SegmentLevel.SUB_PROJECT) + if (existingByName && existingByName.parentSlug === data.parentSlug) { + throw new Error400( + this.options.language, + 'settings.segments.errors.subprojectNameExists', + data.name, + parent.name, + ) + } + const grandparent = await segmentRepository.findBySlug( data.grandparentSlug, SegmentLevel.PROJECT_GROUP, 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.', From 6368682c9418a8161d897e8a2e1bfe62a1d8e699 Mon Sep 17 00:00:00 2001 From: Joana Maia Date: Thu, 18 Dec 2025 21:35:38 +0000 Subject: [PATCH 2/8] chore: add error message to git and gerrit --- backend/src/services/integrationService.ts | 34 +++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/backend/src/services/integrationService.ts b/backend/src/services/integrationService.ts index 6d74c2cb90..75319a39b8 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), From 883a819c1cf0274981cdd7597c134ea7e45c4e5f Mon Sep 17 00:00:00 2001 From: Joana Maia Date: Fri, 19 Dec 2025 14:52:21 +0000 Subject: [PATCH 3/8] chore: add errors to updates as well --- backend/src/services/segmentService.ts | 83 +++++++++++++++++++++++--- 1 file changed, 75 insertions(+), 8 deletions(-) diff --git a/backend/src/services/segmentService.ts b/backend/src/services/segmentService.ts index cc43532ecc..82db8adc93 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 @@ -167,9 +172,9 @@ export default class SegmentService extends LoggerBase { throw new Error(`Project group ${data.parentName} does not exist.`) } - // Check for existing project with same slug in the project group + // Check for existing project with same slug globally const existingBySlug = await segmentRepository.findBySlug(data.slug, SegmentLevel.PROJECT) - if (existingBySlug && existingBySlug.parentSlug === data.parentSlug) { + if (existingBySlug) { throw new Error400( this.options.language, 'settings.segments.errors.projectSlugExists', @@ -178,9 +183,9 @@ export default class SegmentService extends LoggerBase { ) } - // Check for existing project with same name in the project group + // Check for existing project with same name globally const existingByName = await segmentRepository.findByName(data.name, SegmentLevel.PROJECT) - if (existingByName && existingByName.parentSlug === data.parentSlug) { + if (existingByName) { throw new Error400( this.options.language, 'settings.segments.errors.projectNameExists', @@ -254,9 +259,9 @@ export default class SegmentService extends LoggerBase { data.slug = validateNonLfSlug(data.slug) } - // Check for existing subproject with same slug in the project + // Check for existing subproject with same slug globally const existingBySlug = await segmentRepository.findBySlug(data.slug, SegmentLevel.SUB_PROJECT) - if (existingBySlug && existingBySlug.parentSlug === data.parentSlug) { + if (existingBySlug) { throw new Error400( this.options.language, 'settings.segments.errors.subprojectSlugExists', @@ -265,9 +270,9 @@ export default class SegmentService extends LoggerBase { ) } - // Check for existing subproject with same name in the project + // Check for existing subproject with same name globally const existingByName = await segmentRepository.findByName(data.name, SegmentLevel.SUB_PROJECT) - if (existingByName && existingByName.parentSlug === data.parentSlug) { + if (existingByName) { throw new Error400( this.options.language, 'settings.segments.errors.subprojectNameExists', @@ -612,6 +617,68 @@ export default class SegmentService extends LoggerBase { this.setMembersCount(segments, level, membersCountPerSegment) } + private async validateUpdateDuplicates( + segmentId: string, + segment: SegmentData, + data: SegmentUpdateData, + segmentRepository: SegmentRepository + ): Promise { + const segmentType = SegmentService.getSegmentType(segment) + + // Check slug duplication if slug is being updated + if (data.slug && data.slug !== segment.slug) { + const existingBySlug = await segmentRepository.findBySlug(data.slug, segmentType) + + if (existingBySlug?.id !== segmentId) { + + const errorMessage = { + [SegmentLevel.PROJECT_GROUP]: 'settings.segments.errors.projectGroupSlugExists', + [SegmentLevel.PROJECT]: 'settings.segments.errors.projectSlugExists', + [SegmentLevel.SUB_PROJECT]: 'settings.segments.errors.subprojectSlugExists', + } + + throw new Error400( + this.options.language, + errorMessage[segmentType], + data.slug, + ) + } + } + + // Check name duplication if name is being updated + if (data.name && data.name !== segment.name) { + const existingByName = await segmentRepository.findByName(data.name, segmentType) + if (existingByName?.id !== segmentId) { + + const errorMessage = { + [SegmentLevel.PROJECT_GROUP]: 'settings.segments.errors.projectGroupNameExists', + [SegmentLevel.PROJECT]: 'settings.segments.errors.projectNameExists', + [SegmentLevel.SUB_PROJECT]: 'settings.segments.errors.subprojectNameExists', + } + + throw new Error400( + this.options.language, + errorMessage[segmentType], + data.name, + ) + } + } + } + + 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++) { From 2fe56b41e040679e17ae9b50010eeb66d9d08b90 Mon Sep 17 00:00:00 2001 From: Joana Maia Date: Fri, 19 Dec 2025 16:45:22 +0000 Subject: [PATCH 4/8] chore: refactor messages approach --- backend/src/services/segmentService.ts | 203 +++++++++++++------------ 1 file changed, 104 insertions(+), 99 deletions(-) diff --git a/backend/src/services/segmentService.ts b/backend/src/services/segmentService.ts index 82db8adc93..eef41899e3 100644 --- a/backend/src/services/segmentService.ts +++ b/backend/src/services/segmentService.ts @@ -88,25 +88,13 @@ export default class SegmentService extends LoggerBase { const collectionService = new CollectionService({ ...this.options, transaction }) const segmentRepository = new SegmentRepository({ ...this.options, transaction }) - // Check for existing project group with same slug - const existingBySlug = await segmentRepository.findBySlug(data.slug, SegmentLevel.PROJECT_GROUP) - if (existingBySlug) { - throw new Error400( - this.options.language, - 'settings.segments.errors.projectGroupSlugExists', - data.slug, - ) - } - - // Check for existing project group with same name - const existingByName = await segmentRepository.findByName(data.name, SegmentLevel.PROJECT_GROUP) - if (existingByName) { - throw new Error400( - this.options.language, - 'settings.segments.errors.projectGroupNameExists', - data.name, - ) - } + // Check for conflicts with existing segments + await this.validateSegmentConflicts( + segmentRepository, + data.name, + data.slug, + SegmentLevel.PROJECT_GROUP + ) try { // create project group @@ -172,27 +160,13 @@ export default class SegmentService extends LoggerBase { throw new Error(`Project group ${data.parentName} does not exist.`) } - // Check for existing project with same slug globally - const existingBySlug = await segmentRepository.findBySlug(data.slug, SegmentLevel.PROJECT) - if (existingBySlug) { - throw new Error400( - this.options.language, - 'settings.segments.errors.projectSlugExists', - data.slug, - parent.name, - ) - } - - // Check for existing project with same name globally - const existingByName = await segmentRepository.findByName(data.name, SegmentLevel.PROJECT) - if (existingByName) { - throw new Error400( - this.options.language, - 'settings.segments.errors.projectNameExists', - data.name, - parent.name, - ) - } + // Check for conflicts with existing segments + await this.validateSegmentConflicts( + segmentRepository, + data.name, + data.slug, + SegmentLevel.PROJECT + ) if (parent.isLF !== data.isLF) throw new Error400(this.options.language, `settings.segments.errors.isLfNotMatchingParent`) @@ -259,27 +233,13 @@ export default class SegmentService extends LoggerBase { data.slug = validateNonLfSlug(data.slug) } - // Check for existing subproject with same slug globally - const existingBySlug = await segmentRepository.findBySlug(data.slug, SegmentLevel.SUB_PROJECT) - if (existingBySlug) { - throw new Error400( - this.options.language, - 'settings.segments.errors.subprojectSlugExists', - data.slug, - parent.name, - ) - } - - // Check for existing subproject with same name globally - const existingByName = await segmentRepository.findByName(data.name, SegmentLevel.SUB_PROJECT) - if (existingByName) { - throw new Error400( - this.options.language, - 'settings.segments.errors.subprojectNameExists', - data.name, - parent.name, - ) - } + // Check for conflicts with existing segments + await this.validateSegmentConflicts( + segmentRepository, + data.name, + data.slug, + SegmentLevel.SUB_PROJECT + ) const grandparent = await segmentRepository.findBySlug( data.grandparentSlug, @@ -617,6 +577,82 @@ export default class SegmentService extends LoggerBase { this.setMembersCount(segments, level, membersCountPerSegment) } + private async validateSegmentConflicts( + segmentRepository: SegmentRepository, + name?: string, + slug?: string, + segmentType?: SegmentLevel, + excludeId?: string + ): Promise { + if (slug) { + const existingBySlug = await segmentRepository.findBySlug(slug, segmentType) + + if (existingBySlug && (!excludeId || existingBySlug.id !== excludeId)) { + await this.throwSegmentConflictError(segmentRepository, existingBySlug, 'slug', slug) + } + } + + if (name) { + const existingByName = await segmentRepository.findByName(name, segmentType) + + if (existingByName && (!excludeId || existingByName.id !== excludeId)) { + await this.throwSegmentConflictError(segmentRepository, existingByName, 'name', name) + } + } + } + + 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: { + 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' + + // Get the actual parent name from the database + 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' + + // Get the actual parent name from the database + const subprojectParent = await segmentRepository.findById(existingSegment.parentId) + parentName = subprojectParent?.name + break + } + + default: + throw new Error(`Unknown segment type: ${existingSegmentType}`) + } + + if (parentName) { + throw new Error400(this.options.language, errorKey, conflictValue, parentName) + } else { + throw new Error400(this.options.language, errorKey, conflictValue) + } + } + private async validateUpdateDuplicates( segmentId: string, segment: SegmentData, @@ -625,44 +661,13 @@ export default class SegmentService extends LoggerBase { ): Promise { const segmentType = SegmentService.getSegmentType(segment) - // Check slug duplication if slug is being updated - if (data.slug && data.slug !== segment.slug) { - const existingBySlug = await segmentRepository.findBySlug(data.slug, segmentType) - - if (existingBySlug?.id !== segmentId) { - - const errorMessage = { - [SegmentLevel.PROJECT_GROUP]: 'settings.segments.errors.projectGroupSlugExists', - [SegmentLevel.PROJECT]: 'settings.segments.errors.projectSlugExists', - [SegmentLevel.SUB_PROJECT]: 'settings.segments.errors.subprojectSlugExists', - } - - throw new Error400( - this.options.language, - errorMessage[segmentType], - data.slug, - ) - } - } - - // Check name duplication if name is being updated - if (data.name && data.name !== segment.name) { - const existingByName = await segmentRepository.findByName(data.name, segmentType) - if (existingByName?.id !== segmentId) { - - const errorMessage = { - [SegmentLevel.PROJECT_GROUP]: 'settings.segments.errors.projectGroupNameExists', - [SegmentLevel.PROJECT]: 'settings.segments.errors.projectNameExists', - [SegmentLevel.SUB_PROJECT]: 'settings.segments.errors.subprojectNameExists', - } - - throw new Error400( - this.options.language, - errorMessage[segmentType], - data.name, - ) - } - } + await this.validateSegmentConflicts( + segmentRepository, + data.name !== segment.name ? data.name : undefined, + data.slug !== segment.slug ? data.slug : undefined, + segmentType, + segmentId + ) } private static getSegmentType(segment: SegmentData): SegmentLevel { From 9d0281f0bf740026d69adade330f52dedf1fbd1d Mon Sep 17 00:00:00 2001 From: Joana Maia Date: Fri, 19 Dec 2025 17:04:20 +0000 Subject: [PATCH 5/8] chore: support nonlf projects as well --- backend/src/services/segmentService.ts | 74 +++++++++++++++++++++++--- 1 file changed, 66 insertions(+), 8 deletions(-) diff --git a/backend/src/services/segmentService.ts b/backend/src/services/segmentService.ts index eef41899e3..c1999ab4a4 100644 --- a/backend/src/services/segmentService.ts +++ b/backend/src/services/segmentService.ts @@ -93,7 +93,8 @@ export default class SegmentService extends LoggerBase { segmentRepository, data.name, data.slug, - SegmentLevel.PROJECT_GROUP + SegmentLevel.PROJECT_GROUP, + data.isLF ) try { @@ -165,7 +166,8 @@ export default class SegmentService extends LoggerBase { segmentRepository, data.name, data.slug, - SegmentLevel.PROJECT + SegmentLevel.PROJECT, + data.isLF ) if (parent.isLF !== data.isLF) @@ -238,7 +240,8 @@ export default class SegmentService extends LoggerBase { segmentRepository, data.name, data.slug, - SegmentLevel.SUB_PROJECT + SegmentLevel.SUB_PROJECT, + parent.isLF ) const grandparent = await segmentRepository.findBySlug( @@ -577,30 +580,68 @@ 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, - excludeId?: string + isLF?: boolean, + excludeId?: string, ): Promise { + // Validate slug conflicts if slug is provided if (slug) { - const existingBySlug = await segmentRepository.findBySlug(slug, segmentType) + let searchSlug = slug + + // For non-LF projects and sub-projects, slugs are stored with 'nonlf_' prefix + // We need to search for the actual stored format to detect conflicts + if (isLF === false && (segmentType === SegmentLevel.PROJECT || segmentType === SegmentLevel.SUB_PROJECT)) { + searchSlug = slug.startsWith('nonlf_') ? slug : `nonlf_${slug}` + } + + const existingBySlug = await segmentRepository.findBySlug(searchSlug, segmentType) + // If we found a conflicting segment and it's not the one we're updating 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, @@ -614,6 +655,7 @@ export default class SegmentService extends LoggerBase { 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' @@ -625,7 +667,8 @@ export default class SegmentService extends LoggerBase { ? 'settings.segments.errors.projectSlugExists' : 'settings.segments.errors.projectNameExists' - // Get the actual parent name from the database + // 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 @@ -636,7 +679,8 @@ export default class SegmentService extends LoggerBase { ? 'settings.segments.errors.subprojectSlugExists' : 'settings.segments.errors.subprojectNameExists' - // Get the actual parent name from the database + // 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 @@ -646,6 +690,7 @@ export default class SegmentService extends LoggerBase { 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 { @@ -653,6 +698,17 @@ export default class SegmentService extends LoggerBase { } } + /** + * 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, @@ -661,12 +717,14 @@ export default class SegmentService extends LoggerBase { ): 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, - segmentId + data.isLF !== undefined ? data.isLF : segment.isLF, + segmentId, // Exclude the current segment from conflict checks ) } From 322c0b7b98b4f77bdbdbc20b0bd4fa12cc50fc32 Mon Sep 17 00:00:00 2001 From: Joana Maia Date: Fri, 19 Dec 2025 17:21:10 +0000 Subject: [PATCH 6/8] fix: slugs logic --- backend/src/services/segmentService.ts | 36 ++++++++++++++++---------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/backend/src/services/segmentService.ts b/backend/src/services/segmentService.ts index c1999ab4a4..b5f93fd9f1 100644 --- a/backend/src/services/segmentService.ts +++ b/backend/src/services/segmentService.ts @@ -603,19 +603,29 @@ export default class SegmentService extends LoggerBase { ): Promise { // Validate slug conflicts if slug is provided if (slug) { - let searchSlug = slug - - // For non-LF projects and sub-projects, slugs are stored with 'nonlf_' prefix - // We need to search for the actual stored format to detect conflicts - if (isLF === false && (segmentType === SegmentLevel.PROJECT || segmentType === SegmentLevel.SUB_PROJECT)) { - searchSlug = slug.startsWith('nonlf_') ? slug : `nonlf_${slug}` - } - - const existingBySlug = await segmentRepository.findBySlug(searchSlug, segmentType) - - // If we found a conflicting segment and it's not the one we're updating - if (existingBySlug && (!excludeId || existingBySlug.id !== excludeId)) { - await this.throwSegmentConflictError(segmentRepository, existingBySlug, 'slug', 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) + } } } From 3c6dcaebb714c724aa57d9818794a36490c39070 Mon Sep 17 00:00:00 2001 From: Joana Maia Date: Fri, 19 Dec 2025 17:40:22 +0000 Subject: [PATCH 7/8] fix: lint --- .../repositories/gitlabReposRepository.ts | 4 +- backend/src/services/integrationService.ts | 4 +- backend/src/services/segmentService.ts | 63 ++++++++++--------- 3 files changed, 37 insertions(+), 34 deletions(-) diff --git a/backend/src/database/repositories/gitlabReposRepository.ts b/backend/src/database/repositories/gitlabReposRepository.ts index fdce06eb75..227ec8cd20 100644 --- a/backend/src/database/repositories/gitlabReposRepository.ts +++ b/backend/src/database/repositories/gitlabReposRepository.ts @@ -52,7 +52,7 @@ 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( @@ -69,7 +69,7 @@ export default class GitlabReposRepository { 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', diff --git a/backend/src/services/integrationService.ts b/backend/src/services/integrationService.ts index 75319a39b8..85d8343167 100644 --- a/backend/src/services/integrationService.ts +++ b/backend/src/services/integrationService.ts @@ -1394,7 +1394,7 @@ export default class IntegrationService { // 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 @@ -1412,7 +1412,7 @@ export default class IntegrationService { 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', diff --git a/backend/src/services/segmentService.ts b/backend/src/services/segmentService.ts index b5f93fd9f1..fd4d49384a 100644 --- a/backend/src/services/segmentService.ts +++ b/backend/src/services/segmentService.ts @@ -94,7 +94,7 @@ export default class SegmentService extends LoggerBase { data.name, data.slug, SegmentLevel.PROJECT_GROUP, - data.isLF + data.isLF, ) try { @@ -167,7 +167,7 @@ export default class SegmentService extends LoggerBase { data.name, data.slug, SegmentLevel.PROJECT, - data.isLF + data.isLF, ) if (parent.isLF !== data.isLF) @@ -241,7 +241,7 @@ export default class SegmentService extends LoggerBase { data.name, data.slug, SegmentLevel.SUB_PROJECT, - parent.isLF + parent.isLF, ) const grandparent = await segmentRepository.findBySlug( @@ -583,14 +583,14 @@ export default class SegmentService extends LoggerBase { /** * 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( @@ -608,13 +608,13 @@ export default class SegmentService extends LoggerBase { 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)) { @@ -632,7 +632,7 @@ export default class SegmentService extends LoggerBase { // 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) @@ -644,58 +644,61 @@ export default class SegmentService extends LoggerBase { * 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 + 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' + 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' - + 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' - + 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}`) } @@ -711,22 +714,22 @@ export default class SegmentService extends LoggerBase { /** * 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 + segmentRepository: SegmentRepository, ): Promise { const segmentType = SegmentService.getSegmentType(segment) - + // Only validate fields that are actually being changed await this.validateSegmentConflicts( segmentRepository, From 9ff86ddec18984f7836d7f51b7c8b2bc540e0656 Mon Sep 17 00:00:00 2001 From: Joana Maia Date: Fri, 19 Dec 2025 17:51:32 +0000 Subject: [PATCH 8/8] fix: try catch --- backend/src/services/segmentService.ts | 42 ++++++++++++-------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/backend/src/services/segmentService.ts b/backend/src/services/segmentService.ts index fd4d49384a..c4678a8638 100644 --- a/backend/src/services/segmentService.ts +++ b/backend/src/services/segmentService.ts @@ -88,16 +88,15 @@ export default class SegmentService extends LoggerBase { const collectionService = new CollectionService({ ...this.options, transaction }) const segmentRepository = new SegmentRepository({ ...this.options, transaction }) - // Check for conflicts with existing segments - await this.validateSegmentConflicts( - segmentRepository, - data.name, - data.slug, - SegmentLevel.PROJECT_GROUP, - data.isLF, - ) - 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) @@ -161,20 +160,19 @@ export default class SegmentService extends LoggerBase { throw new Error(`Project group ${data.parentName} does not exist.`) } - // 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) - 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 })