Skip to content

update segment service methods for typeorm 1.0 cascading behavior #3253

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

Merged
bcb37 merged 5 commits into from
Jul 31, 2026
Merged

update segment service methods for typeorm 1.0 cascading behavior #3253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2e53ac8
update segment service methods for typeorm 1.0 cascading behavior
bcb37 Jul 28, 2026
1569654
Potential fix for pull request finding
bcb37 Jul 28, 2026
13153ba
more precise type
bcb37 Jul 28, 2026
6d0e591
add relations in one call
bcb37 Jul 28, 2026
da3f898
create new ids for cloned private segments
bcb37 Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 91 additions & 55 deletions packages/backend/src/api/services/SegmentService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ import {
EXPERIMENT_STATE_DISPLAY_NAME_OVERRIDES,
EXPERIMENT_STATE,
} from 'upgrade_types';
import { Not } from 'typeorm';
import { EntityManager, DataSource } from 'typeorm';
import { EntityManager, DataSource, Not, In } from 'typeorm';
import Papa from 'papaparse';
import { env } from '../../env';

Expand Down Expand Up @@ -432,13 +431,27 @@ export class SegmentService {
const newList: SegmentInputValidator = { ...segmentInput, type: SEGMENT_TYPE.PRIVATE };
const createdSegment = await manager.transaction(async (transactionalEntityManager) => {
const createdSegment = await this.upsertSegmentInPipeline(newList, logger, transactionalEntityManager);
const parentSegment = await this.getSegmentById(parentSegmentId, logger);
const segmentRepo = transactionalEntityManager.getRepository(Segment);

// Load only the ID (+ tags) and avoid loading relationships (which can trigger cascade issues)
const parentSegment = await segmentRepo.findOne({
where: { id: parentSegmentId },
select: { id: true, tags: true },
});

if (!parentSegment) {
throw new Error('Parent Segment not found');
}
parentSegment.tags = parentSegment.tags || [];
parentSegment.subSegments = [...parentSegment.subSegments, createdSegment];
await transactionalEntityManager.getRepository(Segment).save(parentSegment);

// Update tags if needed
if (!parentSegment.tags) {
parentSegment.tags = [];
await segmentRepo.save(parentSegment);
}

// Use relation API to avoid cascade behavior from loaded entities
await segmentRepo.createQueryBuilder().relation('subSegments').of(parentSegment).add(createdSegment);

return createdSegment;
});

Expand All @@ -449,24 +462,20 @@ export class SegmentService {

public async deleteList(segmentId: string, parentSegmentId: string, logger: UpgradeLogger): Promise<Segment> {
logger.info({ message: `Deleting list => ${segmentId} from segment ${parentSegmentId}` });

const parentSegment = await this.getSegmentById(parentSegmentId, logger);
if (!parentSegment || !parentSegment.subSegments.some((subSegment) => subSegment.id === segmentId)) {
throw new Error(`List ${segmentId} not found in parent segment ${parentSegmentId}`);
}

const manager = this.dataSource;
const deletedSegmentResponse = await manager.transaction(async (transactionalEntityManager) => {
const parentSegment = await this.getSegmentById(parentSegmentId, logger);
if (!parentSegment) {
throw new Error('Parent Segment not found');
}
if (!parentSegment.subSegments.map((subSegment) => subSegment.id).includes(segmentId)) {
throw new Error(`List ${segmentId} not found in parent segment ${parentSegmentId}`);
}
const deletedSegmentResponse = await this.segmentRepository.deleteSegments(
[segmentId],
logger,
transactionalEntityManager
);

parentSegment.subSegments = parentSegment.subSegments.filter((subSegment) => subSegment.id !== segmentId);

await transactionalEntityManager.getRepository(Segment).save(parentSegment);
return deletedSegmentResponse;
});

Expand Down Expand Up @@ -919,20 +928,36 @@ export class SegmentService {
skipScheduleRecompute = false
): Promise<Segment> {
let segmentDoc: Segment;
const segmentRepo = transactionalEntityManager.getRepository(Segment);

if (segment.id) {
try {
// Full replace: clear members with a single delete-by-segmentId per member table. A per-row
// criteria array (the previous approach) expands into a giant OR predicate that is very slow
// for large lists. The delete is cheap even when there are no members, so we skip the
// pre-SELECT that used to load the full member arrays just to decide whether to delete.
// Full replace: clear members and relationships with a single delete-by-segmentId per table.
// A per-row criteria array (the previous approach) expands into a giant OR predicate that
// is very slow for large lists. The delete is cheap even when there are no members, so we
// skip the pre-SELECT that used to load the full member arrays just to decide whether to delete.

// Load old subsegments to remove them
const oldSegment = await segmentRepo.findOne({
where: { id: segment.id },
relations: { subSegments: true },
});

await Promise.all([
this.individualForSegmentRepository.deleteIndividualForSegmentById(
segment.id,
transactionalEntityManager,
logger
),
this.groupForSegmentRepository.deleteGroupForSegmentById(segment.id, transactionalEntityManager, logger),
// Remove old subsegment relationships
oldSegment && oldSegment.subSegments.length > 0
? segmentRepo
.createQueryBuilder()
.relation('subSegments')
.of({ id: segment.id })
.remove(oldSegment.subSegments)
: Promise.resolve(),
]);
} catch (err) {
const error = err as ErrorWithType;
Expand All @@ -946,55 +971,66 @@ export class SegmentService {
// create/update segment document
segment.id = segment.id || crypto.randomUUID();
const { id, name, description, context, type, listType, tags } = segment;
const segmentsById = await this.getSegmentByIds(segment.subSegmentIds || []);
const allSegments = [...segmentsById, ...(segment.subSegments || [])];
// If the segment is public and there are private subsegments, they are lists - so we need to clone the data
const isListData =
type === SEGMENT_TYPE.PUBLIC && allSegments.some((subSegment) => subSegment.type === SEGMENT_TYPE.PRIVATE);
let subSegmentData;
if (isListData) {

let subSegmentData: Array<Pick<Segment, 'id'>> = [];

// For non-list segments with subSegmentIds, just verify they exist without loading full relations
if (type === SEGMENT_TYPE.PUBLIC && segment.subSegments?.some((sub) => sub.type === SEGMENT_TYPE.PRIVATE)) {
// Public segment with embedded private subsegments (lists) - recursively create them
subSegmentData = await Promise.all(
allSegments.map(async (subSegment) => {
// Create a new segment input object for the list
segment.subSegments.map(async (subSegment) => {
// Create a new segment input object for the list (cloning, not updating)
const segmentInput = subSegment as unknown as SegmentInputValidator;
segmentInput.userIds = subSegment.individualForSegment.map((user) => user.userId);
segmentInput.groups = subSegment.groupForSegment.map((group) => {
return { type: group.type, groupId: group.groupId };
});
segmentInput.subSegmentIds = subSegment.subSegments.map((subSegment) => subSegment.id);
subSegment.id = undefined;
segmentInput.id = undefined; // Clear id to create new clone instead of updating existing segment
segmentInput.userIds = subSegment.individualForSegment?.map((user) => user.userId) || [];
segmentInput.groups =
Comment thread
bcb37 marked this conversation as resolved.
subSegment.groupForSegment?.map((group) => {
return { type: group.type, groupId: group.groupId };
}) || [];
segmentInput.subSegmentIds = subSegment.subSegments?.map((subSegment) => subSegment.id) || [];
return await this.addSegmentDataWithPipeline(segmentInput, logger, transactionalEntityManager);
})
);
} else {
subSegmentData =
segment.subSegmentIds
?.map((subSegmentId) => {
const subSegment = allSegments.find((segment) => subSegmentId === segment.id);
if (subSegment) {
return subSegment;
} else {
const error = new Error(
'SubSegment: ' + subSegmentId + ' not found. Please import subSegment and link in experiment.'
);
(error as any).type = SERVER_ERROR.QUERY_FAILED;
logger.error(error);
return null;
}
})
?.filter((subSegment) => subSegment !== null) || []; // filter out null values
} else if (segment.subSegmentIds && segment.subSegmentIds.length > 0) {
// Just adding references to existing segments - verify existence without loading full relations
const existingIds = await segmentRepo.find({
where: { id: In(segment.subSegmentIds) },
select: { id: true },
});

const existingIdSet = new Set(existingIds.map((s) => s.id));

for (const subSegmentId of segment.subSegmentIds) {
if (!existingIdSet.has(subSegmentId)) {
// Skip unknown subsegment references silently — validation already warned about these
logger.warn({ message: `SubSegment: ${subSegmentId} not found, skipping reference.` });
continue;
}
subSegmentData.push({ id: subSegmentId });
}
}

try {
segmentDoc = await transactionalEntityManager.getRepository(Segment).save({
segmentDoc = await segmentRepo.save({
id,
name,
description,
context,
type,
listType,
tags,
subSegments: subSegmentData,
// Don't include subSegments in save to avoid cascade updates
});

// Manually set the relationship without cascading updates to child segments
// This prevents TypeORM from overwriting the child segments' own subSegments relationships
if (subSegmentData.length > 0) {
await segmentRepo
.createQueryBuilder()
.relation('subSegments')
.of(segmentDoc)
.add(subSegmentData.map((s) => ({ id: s.id })));
}
} catch (err) {
const error = err as ErrorWithType;
error.details = 'Error in saving segment in DB';
Expand Down Expand Up @@ -1054,7 +1090,7 @@ export class SegmentService {
this.featureFlagPrecomputedSegmentService.scheduleRecomputeForSegment(segmentDoc.id, logger);
}

return transactionalEntityManager.getRepository(Segment).findOne({
return segmentRepo.findOne({
where: { id: segmentDoc.id },
relations: { subSegments: true, individualForSegment: true, groupForSegment: true },
});
Expand Down
Loading