fixed segments not being copied (#2105)

* fixed segments not being copied

* fix fmt

* bug fix

* return segmentId[] when getting a feature strategy

* do not return segments if they are not there

* Update src/lib/services/feature-toggle-service.ts

Co-authored-by: Fredrik Strand Oseberg <fredrik.no@gmail.com>

* fix lint

* fix: more explicit column sorting and bug fix

* update snapshot

* rollback

* add segment ids to feature strategies

* bug fix

Co-authored-by: Fredrik Strand Oseberg <fredrik.no@gmail.com>

frontend/src/component/feature/FeatureStrategy/FeatureStrategyEmpty/FeatureStrategyEmpty.tsx

@@ -12,6 +12,8 @@ import { useFeatureImmutable } from 'hooks/api/getters/useFeature/useFeatureImmu
 import { getFeatureStrategyIcon } from 'utils/strategyNames';
 import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
 import { CopyButton } from './CopyButton/CopyButton';
+import { useSegments } from '../../../../hooks/api/getters/useSegments/useSegments';
+import { IFeatureStrategyPayload } from '../../../../interfaces/strategy';
 
 interface IFeatureStrategyEmptyProps {
     projectId: string;
@@ -65,6 +67,7 @@ export const FeatureStrategyEmpty = ({
                     const { id, ...strategyCopy } = {
                         ...strategy,
                         environment: environmentId,
+                        copyOf: strategy.id,
                     };
 
                     return addStrategyToFeature(

frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/CopyStrategyIconMenu/CopyStrategyIconMenu.tsx

@@ -8,7 +8,7 @@ import {
     Tooltip,
 } from '@mui/material';
 import { AddToPhotos as CopyIcon, Lock } from '@mui/icons-material';
-import { IFeatureStrategy } from 'interfaces/strategy';
+import { IFeatureStrategy, IFeatureStrategyPayload } from 'interfaces/strategy';
 import { useRequiredPathParam } from 'hooks/useRequiredPathParam';
 import { IFeatureEnvironment } from 'interfaces/featureToggle';
 import AccessContext from 'contexts/AccessContext';
@@ -19,6 +19,7 @@ import useFeatureStrategyApi from 'hooks/api/actions/useFeatureStrategyApi/useFe
 import useToast from 'hooks/useToast';
 import { useFeatureImmutable } from 'hooks/api/getters/useFeature/useFeatureImmutable';
 import { formatUnknownError } from 'utils/formatUnknownError';
+import { useSegments } from '../../../../../../../../../../hooks/api/getters/useSegments/useSegments';
 
 interface ICopyStrategyIconMenuProps {
     environments: IFeatureEnvironment['name'][];
@@ -31,6 +32,8 @@ export const CopyStrategyIconMenu: VFC<ICopyStrategyIconMenuProps> = ({
 }) => {
     const projectId = useRequiredPathParam('projectId');
     const featureId = useRequiredPathParam('featureId');
+    const { segments } = useSegments(strategy.id);
+
     const [anchorEl, setAnchorEl] = useState<null | HTMLElement>(null);
     const open = Boolean(anchorEl);
     const { addStrategyToFeature } = useFeatureStrategyApi();
@@ -48,6 +51,7 @@ export const CopyStrategyIconMenu: VFC<ICopyStrategyIconMenuProps> = ({
         const { id, ...strategyCopy } = {
             ...strategy,
             environment: environmentId,
+            copyOf: strategy.id,
         };
 
         try {

frontend/src/interfaces/strategy.ts

@@ -19,6 +19,7 @@ export interface IFeatureStrategyPayload {
     name?: string;
     constraints: IConstraint[];
     parameters: IFeatureStrategyParameters;
+    copyOf?: string;
 }
 
 export interface IStrategy {

src/lib/db/environment-store.ts

@@ -101,7 +101,10 @@ export default class EnvironmentStore implements IEnvironmentStore {
     async getAll(query?: Object): Promise<IEnvironment[]> {
         let qB = this.db<IEnvironmentsTable>(TABLE)
             .select('*')
-            .orderBy('sort_order', 'created_at');
+            .orderBy([
+                { column: 'sort_order', order: 'asc' },
+                { column: 'created_at', order: 'asc' },
+            ]);
         if (query) {
             qB = qB.where(query);
         }

src/lib/db/feature-strategy-store.ts

@@ -193,7 +193,10 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
                 feature_name: featureName,
                 environment,
             })
-            .orderBy('sort_order', 'created_at');
+            .orderBy([
+                { column: 'sort_order', order: 'asc' },
+                { column: 'created_at', order: 'asc' },
+            ]);
         stopTimer();
         return rows.map(mapRow);
     }

src/lib/openapi/spec/create-feature-strategy-schema.ts

@@ -19,6 +19,9 @@ export const createFeatureStrategySchema = {
                 $ref: '#/components/schemas/constraintSchema',
             },
         },
+        copyOf: {
+            type: 'string',
+        },
         parameters: {
             $ref: '#/components/schemas/parametersSchema',
         },

src/lib/openapi/spec/public-signup-token-schema.ts

@@ -21,6 +21,8 @@ export const publicSignupTokenSchema = {
             type: 'string',
         },
         url: {
+            description:
+                'The public signup link for the token. Users who follow this link will be taken to a signup page where they can create an Unleash user.',
             type: 'string',
         },
         name: {
@@ -43,12 +45,15 @@ export const publicSignupTokenSchema = {
         },
         users: {
             type: 'array',
+            description: 'Array of users that have signed up using the token',
             items: {
                 $ref: '#/components/schemas/userSchema',
             },
             nullable: true,
         },
         role: {
+            description:
+                'Users who sign up using this token will be given this role.',
             $ref: '#/components/schemas/roleSchema',
         },
     },

src/lib/routes/admin-api/project/features.ts

@@ -39,6 +39,7 @@ import { FeatureEnvironmentSchema } from '../../../openapi/spec/feature-environm
 import { SetStrategySortOrderSchema } from '../../../openapi/spec/set-strategy-sort-order-schema';
 
 import { emptyResponse } from '../../../openapi/util/standard-responses';
+import { SegmentService } from '../../../services/segment-service';
 
 interface FeatureStrategyParams {
     projectId: string;
@@ -68,7 +69,10 @@ const PATH_STRATEGY = `${PATH_STRATEGIES}/:strategyId`;
 
 type ProjectFeaturesServices = Pick<
     IUnleashServices,
-    'featureToggleServiceV2' | 'projectHealthService' | 'openApiService'
+    | 'featureToggleServiceV2'
+    | 'projectHealthService'
+    | 'openApiService'
+    | 'segmentService'
 >;
 
 export default class ProjectFeaturesController extends Controller {
@@ -76,15 +80,22 @@ export default class ProjectFeaturesController extends Controller {
 
     private openApiService: OpenApiService;
 
+    private segmentService: SegmentService;
+
     private readonly logger: Logger;
 
     constructor(
         config: IUnleashConfig,
-        { featureToggleServiceV2, openApiService }: ProjectFeaturesServices,
+        {
+            featureToggleServiceV2,
+            openApiService,
+            segmentService,
+        }: ProjectFeaturesServices,
     ) {
         super(config);
         this.featureService = featureToggleServiceV2;
         this.openApiService = openApiService;
+        this.segmentService = segmentService;
         this.logger = config.getLogger('/admin-api/project/features.ts');
 
         this.route({
@@ -557,13 +568,29 @@ export default class ProjectFeaturesController extends Controller {
         res: Response<FeatureStrategySchema>,
     ): Promise<void> {
         const { projectId, featureName, environment } = req.params;
+        const { copyOf, ...strategyConfig } = req.body;
+
         const userName = extractUsername(req);
         const strategy = await this.featureService.createStrategy(
-            req.body,
+            strategyConfig,
             { environment, projectId, featureName },
             userName,
         );
-        res.status(200).json(strategy);
+
+        if (copyOf) {
+            this.logger.info(
+                `Cloning segments from: strategyId=${copyOf} to: strategyId=${strategy.id} `,
+            );
+            await this.segmentService.cloneStrategySegments(
+                copyOf,
+                strategy.id,
+            );
+        }
+
+        const updatedStrategy = await this.featureService.getStrategy(
+            strategy.id,
+        );
+        res.status(200).json(updatedStrategy);
     }
 
     async getFeatureStrategies(

src/lib/routes/public-invite.ts

@@ -53,6 +53,7 @@ export class PublicInviteController extends Controller {
                 openApiService.validPath({
                     tags: ['Public signup tokens'],
                     operationId: 'validatePublicSignupToken',
+                    summary: `Validates a public signup token exists, has not expired and is enabled`,
                     responses: {
                         200: emptyResponse,
                         ...getStandardResponses(400),
@@ -70,6 +71,8 @@ export class PublicInviteController extends Controller {
                 openApiService.validPath({
                     tags: ['Public signup tokens'],
                     operationId: 'addPublicSignupTokenUser',
+                    summary:
+                        'Create a user with the "viewer" root role and link them to a signup token',
                     requestBody: createRequestSchema('createInvitedUserSchema'),
                     responses: {
                         200: createResponseSchema('userSchema'),

src/lib/services/feature-toggle-service.ts

@@ -45,6 +45,7 @@ import {
     IFeatureOverview,
     IFeatureStrategy,
     IFeatureToggleQuery,
+    ISegment,
     IStrategyConfig,
     IVariant,
     WeightType,
@@ -283,12 +284,14 @@ class FeatureToggleService {
 
     featureStrategyToPublic(
         featureStrategy: IFeatureStrategy,
+        segments: ISegment[] = [],
     ): Saved<IStrategyConfig> {
         return {
             id: featureStrategy.id,
             name: featureStrategy.strategyName,
             constraints: featureStrategy.constraints || [],
             parameters: featureStrategy.parameters,
+            segments: segments.map((segment) => segment.id) ?? [],
         };
     }
 
@@ -330,7 +333,13 @@ class FeatureToggleService {
                 });
 
             const tags = await this.tagStore.getAllTagsForFeature(featureName);
-            const strategy = this.featureStrategyToPublic(newFeatureStrategy);
+            const segments = await this.segmentService.getByStrategy(
+                newFeatureStrategy.id,
+            );
+            const strategy = this.featureStrategyToPublic(
+                newFeatureStrategy,
+                segments,
+            );
             await this.eventStore.store(
                 new FeatureStrategyAddEvent({
                     project: projectId,
@@ -385,10 +394,17 @@ class FeatureToggleService {
                 updates,
             );
 
+            const segments = await this.segmentService.getByStrategy(
+                strategy.id,
+            );
+
             // Store event!
             const tags = await this.tagStore.getAllTagsForFeature(featureName);
-            const data = this.featureStrategyToPublic(strategy);
-            const preData = this.featureStrategyToPublic(existingStrategy);
+            const data = this.featureStrategyToPublic(strategy, segments);
+            const preData = this.featureStrategyToPublic(
+                existingStrategy,
+                segments,
+            );
             await this.eventStore.store(
                 new FeatureStrategyUpdateEvent({
                     project: projectId,
@@ -424,8 +440,14 @@ class FeatureToggleService {
                 existingStrategy,
             );
             const tags = await this.tagStore.getAllTagsForFeature(featureName);
-            const data = this.featureStrategyToPublic(strategy);
-            const preData = this.featureStrategyToPublic(existingStrategy);
+            const segments = await this.segmentService.getByStrategy(
+                strategy.id,
+            );
+            const data = this.featureStrategyToPublic(strategy, segments);
+            const preData = this.featureStrategyToPublic(
+                existingStrategy,
+                segments,
+            );
             await this.eventStore.store(
                 new FeatureStrategyUpdateEvent({
                     featureName,
@@ -488,6 +510,7 @@ class FeatureToggleService {
         featureName: string,
         environment: string = DEFAULT_ENV,
     ): Promise<Saved<IStrategyConfig>[]> {
+        this.logger.debug('getStrategiesForEnvironment');
         const hasEnv = await this.featureEnvironmentStore.featureHasEnvironment(
             environment,
             featureName,
@@ -499,13 +522,22 @@ class FeatureToggleService {
                     featureName,
                     environment,
                 );
-            return featureStrategies.map((strat) => ({
+            const result = [];
+            for (const strat of featureStrategies) {
+                const segments =
+                    (await this.segmentService.getByStrategy(strat.id)).map(
+                        (segment) => segment.id,
+                    ) ?? [];
+                result.push({
                     id: strat.id,
                     name: strat.strategyName,
                     constraints: strat.constraints,
                     parameters: strat.parameters,
                     sortOrder: strat.sortOrder,
-            }));
+                    segments,
+                });
+            }
+            return result;
         }
         throw new NotFoundError(
             `Feature ${featureName} does not have environment ${environment}`,
@@ -727,12 +759,23 @@ class FeatureToggleService {
         const strategy = await this.featureStrategiesStore.getStrategyById(
             strategyId,
         );
-        return {
+
+        const segments = await this.segmentService.getByStrategy(strategyId);
+        let result: Saved<IStrategyConfig> = {
             id: strategy.id,
             name: strategy.strategyName,
             constraints: strategy.constraints || [],
             parameters: strategy.parameters,
+            segments: [],
         };
+
+        if (segments && segments.length > 0) {
+            result = {
+                ...result,
+                segments: segments.map((segment) => segment.id),
+            };
+        }
+        return result;
     }
 
     async getEnvironmentInfo(

src/lib/services/segment-service.ts

@@ -124,7 +124,6 @@ export class SegmentService {
         const sourceStrategySegments = await this.getByStrategy(
             sourceStrategyId,
         );
-
         await Promise.all(
             sourceStrategySegments.map((sourceStrategySegment) => {
                 return this.addToStrategy(

src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap

@@ -709,6 +709,9 @@ exports[`should serve the OpenAPI spec 1`] = `
             },
             "type": "array",
           },
+          "copyOf": {
+            "type": "string",
+          },
           "name": {
             "type": "string",
           },
@@ -2483,14 +2486,17 @@ exports[`should serve the OpenAPI spec 1`] = `
           },
           "role": {
             "$ref": "#/components/schemas/roleSchema",
+            "description": "Users who sign up using this token will be given this role.",
           },
           "secret": {
             "type": "string",
           },
           "url": {
+            "description": "The public signup link for the token. Users who follow this link will be taken to a signup page where they can create an Unleash user.",
             "type": "string",
           },
           "users": {
+            "description": "Array of users that have signed up using the token",
             "items": {
               "$ref": "#/components/schemas/userSchema",
             },
@@ -7472,6 +7478,7 @@ If the provided project does not exist, the list of events will be empty.",
             "description": "The provided resource can not be created or updated because it would conflict with the current state of the resource or with an already existing resource, respectively.",
           },
         },
+        "summary": "Create a user with the "viewer" root role and link them to a signup token",
         "tags": [
           "Public signup tokens",
         ],
@@ -7498,6 +7505,7 @@ If the provided project does not exist, the list of events will be empty.",
             "description": "The request data does not match what we expect.",
           },
         },
+        "summary": "Validates a public signup token exists, has not expired and is enabled",
         "tags": [
           "Public signup tokens",
         ],