importing multiple segments at once (#954)

* importing multiple segments at once * resolving testcases * replace segment in importSegment with segments as plural * remove calling query in import segment loop and change in logic * resolve failing testcases * resolve review comments * resolve review comments
CarnegieLearningWeb · Aug 22, 2023 · 9a79b8e · 9a79b8e
1 parent e2d93a2
commit 9a79b8e
Show file tree

Hide file tree

Showing 23 changed files with 345 additions and 153 deletions.
diff --git a/backend/packages/Upgrade/src/api/controllers/SegmentController.ts b/backend/packages/Upgrade/src/api/controllers/SegmentController.ts
@@ -362,23 +362,26 @@ export class SegmentController {
    *          description: Internal Server Error, Insert Error in database, SegmentId is not valid, JSON format is not valid
    */
   @Post('/import')
-  public importSegment(
-    @Body({ validate: { validationError: { target: false, value: false } } }) segment: SegmentInputValidator,
+  public importSegments(
+    @Body({ validate: false }) segment: SegmentInputValidator[],
     @Req() request: AppRequest
-  ): Promise<Segment> {
-    return this.segmentService.importSegment(segment, request.logger);
+  ): Promise<Segment[]> {
+    return this.segmentService.importSegments(segment, request.logger);
   }
 
   @Post('/export')
-  public exportSegments( @Body({ validate: false }) ids: string[], @Req() request: AppRequest): Promise<Segment[]> {
+  public exportSegments(@Body({ validate: false }) ids: string[], @Req() request: AppRequest): Promise<Segment[]> {
     if (!ids) {
       return Promise.reject(new Error(SERVER_ERROR.MISSING_PARAMS + ' : segmentId should not be null.'));
     }
     for (const id of ids) {
       if (!isUUID(id)) {
         return Promise.reject(
           new Error(
-            JSON.stringify({ type: SERVER_ERROR.INCORRECT_PARAM_FORMAT, message: ' : segmentId should be of type UUID.' })
+            JSON.stringify({
+              type: SERVER_ERROR.INCORRECT_PARAM_FORMAT,
+              message: ' : segmentId should be of type UUID.',
+            })
           )
         );
       }

diff --git a/backend/packages/Upgrade/src/api/services/SegmentService.ts b/backend/packages/Upgrade/src/api/services/SegmentService.ts
@@ -152,54 +152,68 @@ export class SegmentService {
     return await this.segmentRepository.deleteSegment(id, logger);
   }
 
-  public async importSegment(segment: SegmentInputValidator, logger: UpgradeLogger): Promise<Segment> {
-    const duplicateSegment = await this.segmentRepository.findOne(segment.id);
-    if (duplicateSegment && segment.id !== undefined) {
-      const error = new Error('Duplicate segment');
-      (error as any).type = SERVER_ERROR.QUERY_FAILED;
-      logger.error(error);
-      throw error;
-    }
+  public async importSegments(segments: SegmentInputValidator[], logger: UpgradeLogger): Promise<Segment[]> {
+    const allAddedSegments: Segment[] = [];
+    const allSegmentIds: string[] = [];
+    segments.forEach((segment) => {
+      allSegmentIds.push(segment.id);
+      segment.subSegmentIds.forEach((subSegment) => {
+        allSegmentIds.includes(subSegment) ? true : allSegmentIds.push(subSegment);
+      });
+    });
+    const allSegmentsData = await this.getSegmentByIds(allSegmentIds);
+    const duplicateSegmentsIds = allSegmentsData?.map((segment) => segment?.id);
 
-    // check for each subSegment to exists
-    const allSegments = await this.segmentRepository.getAllSegments(logger);
-    segment.subSegmentIds.forEach((subSegmentId) => {
-      const subSegment = allSegments.find((segmentId) => subSegmentId === segmentId.id);
-      if (!subSegment) {
-        const error = new Error(
-          'SubSegment: ' + subSegmentId + ' not found. Please import subSegment and link in experiment.'
-        );
+    for (const segment of segments) {
+      const duplicateSegment = duplicateSegmentsIds ? duplicateSegmentsIds.includes(segment.id) : false;
+      if (duplicateSegment && segment.id !== undefined) {
+        const error = new Error('Duplicate segment');
         (error as any).type = SERVER_ERROR.QUERY_FAILED;
         logger.error(error);
         throw error;
       }
-    });
 
-    logger.info({ message: `Import segment => ${JSON.stringify(segment, undefined, 2)}` });
-    return this.addSegmentDataInDB(segment, logger);
+      segment.subSegmentIds.forEach((subSegmentId) => {
+        const subSegment = allSegmentsData ? allSegmentsData.find((segment) => subSegmentId === segment?.id) : null;
+        if (!subSegment) {
+          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);
+          throw error;
+        }
+      });
+
+      logger.info({ message: `Import segment => ${JSON.stringify(segment, undefined, 2)}` });
+      const addedSegment = await this.addSegmentDataInDB(segment, logger);
+      allAddedSegments.push(addedSegment);
+      allSegmentsData.push(addedSegment);
+    }
+    return allAddedSegments;
   }
 
   public async exportSegments(segmentIds: string[], logger: UpgradeLogger): Promise<Segment[]> {
     logger.info({ message: `Export segment by id. segmentId: ${segmentIds}` });
     let segmentsDoc: Segment[] = [];
     if (segmentIds.length > 1) {
       segmentsDoc = await this.getSegmentByIds(segmentIds);
-    }else {
+    } else {
       const segmentDoc = await this.segmentRepository.findOne({
         where: { id: segmentIds[0] },
         relations: ['individualForSegment', 'groupForSegment', 'subSegments'],
       });
       if (!segmentDoc) {
         throw new Error(SERVER_ERROR.QUERY_FAILED);
-      }else {
+      } else {
         segmentsDoc.push(segmentDoc);
       }
     }
 
     return segmentsDoc;
   }
 
-  private async addSegmentDataInDB(segment: SegmentInputValidator, logger: UpgradeLogger): Promise<Segment> {
+  async addSegmentDataInDB(segment: SegmentInputValidator, logger: UpgradeLogger): Promise<Segment> {
     const createdSegment = await getConnection().transaction(async (transactionalEntityManager) => {
       let segmentDoc: Segment;
 

diff --git a/backend/packages/Upgrade/test/unit/services/SegmentService.test.ts b/backend/packages/Upgrade/test/unit/services/SegmentService.test.ts
@@ -20,6 +20,7 @@ import { Experiment } from '../../../src/api/models/Experiment';
 const exp = new Experiment();
 const seg2 = new Segment();
 const seg1 = new Segment();
+const segValSegment = new Segment();
 const logger = new UpgradeLogger();
 const segmentArr = [seg1, seg2];
 const segVal = new SegmentInputValidator();
@@ -67,6 +68,17 @@ describe('Segment Service Testing', () => {
     segVal.subSegmentIds = ['seg1', 'seg2'];
     segVal.userIds = ['user1', 'user2', 'user3'];
     segVal.groups = [{ groupId: 'group1', type: 'type1' }];
+    segValSegment.id = 'segval1';
+    segValSegment.subSegments = [seg1, seg2];
+    const group1 = new GroupForSegment();
+    group1.groupId = 'group1';
+    group1.type = 'type1';
+    segValSegment.groupForSegment = [group1];
+    const user2 = new IndividualForSegment();
+    user2.userId = 'user2';
+    const user3 = new IndividualForSegment();
+    user3.userId = 'user3';
+    segValSegment.individualForSegment = [user, user2, user3];
 
     module = await Test.createTestingModule({
       providers: [
@@ -271,9 +283,7 @@ describe('Segment Service Testing', () => {
     segment.userIds = [];
     segment.groups = [];
     repo.findOne = jest.fn().mockResolvedValue(seg2);
-    const indivRepo = module.get<IndividualForSegmentRepository>(
-      getRepositoryToken(IndividualForSegmentRepository)
-    );
+    const indivRepo = module.get<IndividualForSegmentRepository>(getRepositoryToken(IndividualForSegmentRepository));
     indivRepo.insertIndividualForSegment = jest.fn().mockImplementation(() => {
       throw err;
     });
@@ -308,22 +318,23 @@ describe('Segment Service Testing', () => {
   });
 
   it('should import a segment', async () => {
-    repo.findOne = jest.fn().mockResolvedValue(null);
-    const segments = await service.importSegment(segVal, logger);
-    expect(segments).toEqual(null);
+    service.getSegmentByIds = jest.fn().mockResolvedValue([seg1, seg2]);
+    service.addSegmentDataInDB = jest.fn().mockResolvedValue(segValSegment);
+    const segments = await service.importSegments([segVal], logger);
+    expect(segments).toEqual([segValSegment]);
   });
 
   it('should throw an error when trying to import a duplicate segment', async () => {
+    service.getSegmentByIds = jest.fn().mockResolvedValue([seg1, seg2, segVal]);
     expect(async () => {
-      await service.importSegment(segVal, logger);
+      await service.importSegments([segVal], logger);
     }).rejects.toThrow(new Error('Duplicate segment'));
   });
 
   it('should throw an error when trying to import a segment that includes an unknown subsegment', async () => {
-    repo.findOne = jest.fn().mockResolvedValue(null);
-    repo.getAllSegments = jest.fn().mockResolvedValue([seg1]);
+    service.getSegmentByIds = jest.fn().mockResolvedValue([seg1]);
     expect(async () => {
-      await service.importSegment(segVal, logger);
+      await service.importSegments([segVal], logger);
     }).rejects.toThrow(
       new Error('SubSegment: ' + seg2.id + ' not found. Please import subSegment and link in experiment.')
     );

diff --git a/frontend/projects/upgrade/src/app/core/segments/segments.data.service.spec.ts b/frontend/projects/upgrade/src/app/core/segments/segments.data.service.spec.ts
@@ -95,14 +95,14 @@ describe('SegmentDataService', () => {
     });
   });
 
-  describe('#importSegment', () => {
-    it('should get the importSegment http observable', () => {
-      const mockUrl = mockEnvironment.api.importSegment;
+  describe('#importSegments', () => {
+    it('should get the importSegments http observable', () => {
+      const mockUrl = mockEnvironment.api.importSegments;
       const segment = { ...mockSegment };
 
-      service.importSegment(segment);
+      service.importSegments([segment]);
 
-      expect(mockHttpClient.post).toHaveBeenCalledWith(mockUrl, { ...segment });
+      expect(mockHttpClient.post).toHaveBeenCalledWith(mockUrl, [segment]);
     });
   });
 });
diff --git a/frontend/projects/upgrade/src/app/core/segments/segments.data.service.ts b/frontend/projects/upgrade/src/app/core/segments/segments.data.service.ts
@@ -32,8 +32,8 @@ export class SegmentsDataService {
     return this.http.post(url, segmentIds);
   }
 
-  importSegment(segment: SegmentInput) {
-    const url = this.environment.api.importSegment;
-    return this.http.post(url, { ...segment });
+  importSegments(segments: SegmentInput[]) {
+    const url = this.environment.api.importSegments;
+    return this.http.post(url, segments);
   }
 }
diff --git a/frontend/projects/upgrade/src/app/core/segments/segments.service.spec.ts b/frontend/projects/upgrade/src/app/core/segments/segments.service.spec.ts
@@ -2,7 +2,12 @@ import { fakeAsync, tick } from '@angular/core/testing';
 import { BehaviorSubject } from 'rxjs';
 import { SEGMENT_TYPE } from 'upgrade_types';
 import { SegmentsService } from './segments.service';
-import { actionDeleteSegment, actionExportSegments, actionUpsertSegment } from './store/segments.actions';
+import {
+  actionDeleteSegment,
+  actionExportSegments,
+  actionImportSegments,
+  actionUpsertSegment,
+} from './store/segments.actions';
 import { SegmentInput, UpsertSegmentType } from './store/segments.model';
 import * as SegmentSelectors from './store/segments.selectors';
 const MockStateStore$ = new BehaviorSubject({});
@@ -67,16 +72,15 @@ describe('SegmentService', () => {
     });
   });
 
-  describe('#importSegment', () => {
+  describe('#importSegments', () => {
     it('should dispatch actionUpsertExperiment with the given input', () => {
       const segment = { ...mockSegment };
 
-      service.importSegment(segment);
+      service.importSegments([segment]);
 
       expect(mockStore.dispatch).toHaveBeenCalledWith(
-        actionUpsertSegment({
-          segment,
-          actionType: UpsertSegmentType.IMPORT_SEGMENT,
+        actionImportSegments({
+          segments: [segment],
         })
       );
     });

diff --git a/frontend/projects/upgrade/src/app/core/segments/segments.service.ts b/frontend/projects/upgrade/src/app/core/segments/segments.service.ts
@@ -65,9 +65,7 @@ export class SegmentsService {
     this.store$.dispatch(SegmentsActions.actionExportSegments({ segmentIds }));
   }
 
-  importSegment(segment: SegmentInput) {
-    this.store$.dispatch(
-      SegmentsActions.actionUpsertSegment({ segment, actionType: UpsertSegmentType.IMPORT_SEGMENT })
-    );
+  importSegments(segments: SegmentInput[]) {
+    this.store$.dispatch(SegmentsActions.actionImportSegments({ segments }));
   }
 }
diff --git a/frontend/projects/upgrade/src/app/core/segments/store/segments.actions.ts b/frontend/projects/upgrade/src/app/core/segments/store/segments.actions.ts
@@ -26,6 +26,15 @@ export const actionUpsertSegmentSuccess = createAction(
 
 export const actionUpsertSegmentFailure = createAction('[Segments] Upsert Segment Failure');
 
+export const actionImportSegments = createAction('[Segments] Import Segment', props<{ segments: SegmentInput[] }>());
+
+export const actionImportSegmentSuccess = createAction(
+  '[Segments] Import Segment Success',
+  props<{ segments: Segment[] }>()
+);
+
+export const actionImportSegmentFailure = createAction('[Segments] Import Segment Failure');
+
 export const actionGetSegmentById = createAction('[Experiment] Get Segment By Id', props<{ segmentId: string }>());
 
 export const actionGetSegmentByIdSuccess = createAction(

diff --git a/frontend/projects/upgrade/src/app/core/segments/store/segments.effects.spec.ts b/frontend/projects/upgrade/src/app/core/segments/store/segments.effects.spec.ts
@@ -138,35 +138,33 @@ describe('SegmentsEffects', () => {
       tick(0);
     }));
 
-    it('should call importSegment(), no router nav, and call actionUpsertSegmentSuccess if IMPORT_SEGMENT', fakeAsync(() => {
-      segmentsDataService.importSegment = jest.fn().mockReturnValue(of({ ...mockSegment }));
+    it('should call updateSegment(), no router nav, and call actionUpsertSegmentSuccess if UPDATE_SEGMENT', fakeAsync(() => {
+      segmentsDataService.updateSegment = jest.fn().mockReturnValue(of({ ...mockSegment }));
 
       const expectedAction = SegmentsActions.actionUpsertSegmentSuccess({
         segment: { ...mockSegment },
       });
 
       service.upsertSegment$.subscribe((result) => {
         expect(result).toEqual(expectedAction);
-        expect(segmentsDataService.importSegment).toHaveBeenCalled();
+        expect(segmentsDataService.updateSegment).toHaveBeenCalled();
         expect(router.navigate).not.toHaveBeenCalledWith(['/segments']);
       });
 
       actions$.next(
         SegmentsActions.actionUpsertSegment({
           segment: { ...mockSegmentInput },
-          actionType: UpsertSegmentType.IMPORT_SEGMENT,
+          actionType: UpsertSegmentType.UPDATE_SEGMENT,
         })
       );
 
       tick(0);
     }));
 
-    it('should call updateSegment(), no router nav, and call actionUpsertSegmentSuccess if UPDATE_SEGMENT', fakeAsync(() => {
-      segmentsDataService.updateSegment = jest.fn().mockReturnValue(of({ ...mockSegment }));
+    it('should dispatch actionUpsertSegmentFailure on API error', fakeAsync(() => {
+      segmentsDataService.updateSegment = jest.fn().mockReturnValue(throwError(() => new Error('test')));
 
-      const expectedAction = SegmentsActions.actionUpsertSegmentSuccess({
-        segment: { ...mockSegment },
-      });
+      const expectedAction = SegmentsActions.actionUpsertSegmentFailure();
 
       service.upsertSegment$.subscribe((result) => {
         expect(result).toEqual(expectedAction);
@@ -183,22 +181,43 @@ describe('SegmentsEffects', () => {
 
       tick(0);
     }));
+  });
 
-    it('should dispatch actionUpsertSegmentFailure on API error', fakeAsync(() => {
-      segmentsDataService.updateSegment = jest.fn().mockReturnValue(throwError(() => new Error('test')));
+  describe('importSegments$', () => {
+    it('should do nothing if Segment is falsey', fakeAsync(() => {
+      let neverEmitted = true;
 
-      const expectedAction = SegmentsActions.actionUpsertSegmentFailure();
+      service.importSegments$.subscribe(() => {
+        neverEmitted = false;
+      });
+
+      actions$.next(
+        SegmentsActions.actionImportSegments({
+          segments: undefined,
+        })
+      );
+
+      tick(0);
+
+      expect(neverEmitted).toEqual(true);
+    }));
+
+    it('should call importSegments(), no router nav, and call actionUpsertSegmentSuccess if IMPORT_SEGMENT', fakeAsync(() => {
+      segmentsDataService.importSegments = jest.fn().mockReturnValue(of([mockSegment]));
+
+      const expectedAction = SegmentsActions.actionImportSegmentSuccess({
+        segments: [mockSegment],
+      });
 
       service.upsertSegment$.subscribe((result) => {
         expect(result).toEqual(expectedAction);
-        expect(segmentsDataService.updateSegment).toHaveBeenCalled();
+        expect(segmentsDataService.importSegments).toHaveBeenCalled();
         expect(router.navigate).not.toHaveBeenCalledWith(['/segments']);
       });
 
       actions$.next(
-        SegmentsActions.actionUpsertSegment({
-          segment: { ...mockSegmentInput },
-          actionType: UpsertSegmentType.UPDATE_SEGMENT,
+        SegmentsActions.actionImportSegments({
+          segments: [mockSegmentInput],
         })
       );
 
@@ -248,7 +267,7 @@ describe('SegmentsEffects', () => {
         expect(result).toEqual(expectedAction);
       });
 
-      actions$.next(SegmentsActions.actionExportSegments({ segmentIds: [{ ...mockSegment }.id] } ));
+      actions$.next(SegmentsActions.actionExportSegments({ segmentIds: [{ ...mockSegment }.id] }));
 
       tick(0);
     }));