Skip to content

Commit

Permalink
chore(edit-ema) no call to verify experiment if the id is undefined (#…
Browse files Browse the repository at this point in the history
…30676)

### Proposed Changes
This pull request includes several updates to the
`DotExperimentsService` and its related test files to improve error
handling and testing capabilities. Additionally, it includes minor code
cleanup in the `withLoad.ts` file.

Improvements to `DotExperimentsService`:

*
[`core-web/libs/data-access/src/lib/dot-experiments/dot-experiments.service.ts`](diffhunk://#diff-a19f02782cf591f011530ab631225e33c33dc9127c6cffaf9e5ed45a68ae3faaL1-R6):
Updated the `getById` method to handle `undefined` experiment IDs and to
return an `Observable` of `undefined` in case of errors.
[[1]](diffhunk://#diff-a19f02782cf591f011530ab631225e33c33dc9127c6cffaf9e5ed45a68ae3faaL1-R6)
[[2]](diffhunk://#diff-a19f02782cf591f011530ab631225e33c33dc9127c6cffaf9e5ed45a68ae3faaL86-R96)

Enhancements to testing:

*
[`core-web/libs/data-access/src/lib/dot-experiments/dot-experiments.service.spec.ts`](diffhunk://#diff-ae290cc9ac86bc4cd1b54f5c101d820bcf7d4afa64eae76cc9fa849429132bf2R1-R2):
Added imports for `HttpClientTestingModule` and `HttpTestingController`,
and updated the test setup to use `TestBed` for dependency injection.
Added a new test case to handle `undefined` experiment IDs.
[[1]](diffhunk://#diff-ae290cc9ac86bc4cd1b54f5c101d820bcf7d4afa64eae76cc9fa849429132bf2R1-R2)
[[2]](diffhunk://#diff-ae290cc9ac86bc4cd1b54f5c101d820bcf7d4afa64eae76cc9fa849429132bf2R26-R43)
[[3]](diffhunk://#diff-ae290cc9ac86bc4cd1b54f5c101d820bcf7d4afa64eae76cc9fa849429132bf2R191-R197)

Code cleanup:

*
[`core-web/libs/portlets/edit-ema/portlet/src/lib/store/features/load/withLoad.ts`](diffhunk://#diff-0cbc04b5047422551dde0970e4ea1d0e1fcce3d9d8f4e0b4612fbc30b1658487L4-R16):
Reorganized import statements to follow a consistent order and removed
unnecessary `catchError` in the `experiment` observable.
[[1]](diffhunk://#diff-0cbc04b5047422551dde0970e4ea1d0e1fcce3d9d8f4e0b4612fbc30b1658487L4-R16)
[[2]](diffhunk://#diff-0cbc04b5047422551dde0970e4ea1d0e1fcce3d9d8f4e0b4612fbc30b1658487L112-R113)

### Checklist
- [x] Tests
- [ ] Translations
- [ ] Security Implications Contemplated (add notes if applicable)
  • Loading branch information
oidacra authored Nov 15, 2024
1 parent 8912af1 commit 52a1ac9
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -170,4 +170,11 @@ describe('DotExperimentsService', () => {

expect(req.request.body['trafficProportion']).toEqual(newValue);
});

it('should return an Observable of undefined when experimentId is undefined', (done) => {
spectator.service.getById(undefined).subscribe((result) => {
expect(result).toBeUndefined();
done();
});
});
});
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { Observable } from 'rxjs';
import { Observable, of } from 'rxjs';

import { HttpClient } from '@angular/common/http';
import { Injectable } from '@angular/core';

import { pluck } from 'rxjs/operators';
import { catchError, pluck } from 'rxjs/operators';

import { DotCMSResponse } from '@dotcms/dotcms-js';
import {
Expand Down Expand Up @@ -83,13 +83,18 @@ export class DotExperimentsService {
/**
* Get details of an experiment
* @param {string} experimentId
* @returns Observable<DotExperiment>
* @returns Observable<DotExperiment | undefined>
* @memberof DotExperimentsService
*/
getById(experimentId: string): Observable<DotExperiment> {
getById(experimentId: string | undefined): Observable<DotExperiment | undefined> {
if (!experimentId) {
return of(undefined);
}

return this.http
.get<DotCMSResponseExperiment<DotExperiment>>(`${API_ENDPOINT}/${experimentId}`)
.pipe(pluck('entity'));
.pipe(pluck('entity'))
.pipe(catchError(() => of(undefined)));
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
import { tapResponse } from '@ngrx/component-store';
import { patchState, signalStoreFeature, type, withMethods } from '@ngrx/signals';
import { rxMethod } from '@ngrx/signals/rxjs-interop';
import { pipe, forkJoin, of, EMPTY } from 'rxjs';
import { EMPTY, forkJoin, of, pipe } from 'rxjs';

import { HttpErrorResponse } from '@angular/common/http';
import { inject } from '@angular/core';
import { Router, ActivatedRoute } from '@angular/router';
import { ActivatedRoute, Router } from '@angular/router';

import { switchMap, shareReplay, catchError, tap, take, map } from 'rxjs/operators';
import { map, shareReplay, switchMap, take, tap } from 'rxjs/operators';

import { DotLanguagesService, DotLicenseService, DotExperimentsService } from '@dotcms/data-access';
import { DotExperimentsService, DotLanguagesService, DotLicenseService } from '@dotcms/data-access';
import { LoginService } from '@dotcms/dotcms-js';
import { DEFAULT_VARIANT_ID } from '@dotcms/dotcms-models';

import { DotPageApiService, DotPageApiParams } from '../../../services/dot-page-api.service';
import { DotPageApiParams, DotPageApiService } from '../../../services/dot-page-api.service';
import { UVE_STATUS } from '../../../shared/enums';
import { computeCanEditPage, computePageIsLocked, isForwardOrPage } from '../../../utils';
import { UVEState } from '../../models';
Expand Down Expand Up @@ -109,13 +109,9 @@ export function withLoad() {
}),
switchMap(({ pageAPIResponse, isEnterprise, currentUser }) =>
forkJoin({
experiment: dotExperimentsService
.getById(params.experimentId)
.pipe(
// If there is an error, we return undefined
// This is to avoid blocking the page if there is an error with the experiment
catchError(() => of(undefined))
),
experiment: dotExperimentsService.getById(
params.experimentId
),
languages: dotLanguagesService.getLanguagesUsedPage(
pageAPIResponse.page.identifier
)
Expand Down

0 comments on commit 52a1ac9

Please sign in to comment.