Skip to content

Commit

Permalink
fix: do not allow segment deletion when used in private projects (#5236)
Browse files Browse the repository at this point in the history
  • Loading branch information
sjaanus authored Nov 1, 2023
1 parent 74bbc77 commit 598d022
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 9 deletions.
8 changes: 5 additions & 3 deletions src/lib/features/segment/segment-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,10 @@ export class SegmentsController extends Controller {
): Promise<void> {
const { id } = req.params;
const { user } = req;
const strategies = await this.segmentService.getStrategies(id, user.id);
const strategies = await this.segmentService.getVisibleStrategies(
id,
user.id,
);

// Remove unnecessary IFeatureStrategy fields from the response.
const segmentStrategies = strategies.map((strategy) => ({
Expand All @@ -369,8 +372,7 @@ export class SegmentsController extends Controller {
res: Response,
): Promise<void> {
const id = Number(req.params.id);
const { user } = req;
const strategies = await this.segmentService.getStrategies(id, user.id);
const strategies = await this.segmentService.getAllStrategies(id);

if (strategies.length > 0) {
res.status(409).send();
Expand Down
12 changes: 11 additions & 1 deletion src/lib/segments/segment-service-interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,17 @@ export interface ISegmentService {

get(id: number): Promise<ISegment>;

getStrategies(id: number, userId: number): Promise<IFeatureStrategy[]>;
/**
* Gets all strategies for a segment
* This is NOT considering the private projects
* For most use cases, use `getVisibleStrategies`
*/
getAllStrategies(id: number): Promise<IFeatureStrategy[]>;

getVisibleStrategies(
id: number,
userId: number,
): Promise<IFeatureStrategy[]>;

validateName(name: string): Promise<void>;

Expand Down
13 changes: 8 additions & 5 deletions src/lib/services/segment-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,18 +77,15 @@ export class SegmentService implements ISegmentService {
return this.segmentStore.getActiveForClient();
}

// Used by unleash-enterprise.
async getByStrategy(strategyId: string): Promise<ISegment[]> {
return this.segmentStore.getByStrategy(strategyId);
}

// Used by unleash-enterprise.
async getStrategies(
async getVisibleStrategies(
id: number,
userId: number,
): Promise<IFeatureStrategy[]> {
const strategies =
await this.featureStrategiesStore.getStrategiesBySegment(id);
const strategies = await this.getAllStrategies(id);
if (this.flagResolver.isEnabled('privateProjects')) {
const accessibleProjects =
await this.privateProjectChecker.getUserAccessibleProjects(
Expand All @@ -105,6 +102,12 @@ export class SegmentService implements ISegmentService {
return strategies;
}

async getAllStrategies(id: number): Promise<IFeatureStrategy[]> {
const strategies =
await this.featureStrategiesStore.getStrategiesBySegment(id);
return strategies;
}

async create(
data: unknown,
user: Partial<Pick<User, 'username' | 'email'>>,
Expand Down

0 comments on commit 598d022

Please sign in to comment.