Skip to content

Commit

Permalink
fix(uve): Fixed API button on toolbar (#31136)
Browse files Browse the repository at this point in the history 
https://github.com/user-attachments/assets/d480d0d1-ad90-48c5-8e81-5c1480b00f6f




This pull request introduces several changes to improve URL sanitization
and testing within the `dot-ema-shell` component and related files. The
most important changes include adding URL sanitization logic, updating
tests to reflect these changes, and removing redundant code.

### URL Sanitization:

*
[`core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts`](diffhunk://#diff-677330662fea6dadc7e48fd8455ec2a6fe60d624c7ed1f01f0a3e985aacd05c6R213-R215):
Added the `sanitizeURL` function to sanitize URLs before further
processing.
*
[`core-web/libs/portlets/edit-ema/portlet/src/lib/services/guards/edit-ema.guard.ts`](diffhunk://#diff-880595c3f64b60b4dee196fb13dc679b5a500aede2d23cf966a369a70326dddcL5):
Removed the `sanitizeURL` import and related code, as the sanitization
is now handled elsewhere.
[[1]](diffhunk://#diff-880595c3f64b60b4dee196fb13dc679b5a500aede2d23cf966a369a70326dddcL5)
[[2]](diffhunk://#diff-880595c3f64b60b4dee196fb13dc679b5a500aede2d23cf966a369a70326dddcL47-L60)

### Testing Updates:

*
[`core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts`](diffhunk://#diff-8843e3a4ce8c16e83408b1a6dcc3ad54eaddd17f8e986bbdb502e11bd4446ab4R315-R391):
Added new test cases to verify URL sanitization when `loadPageAsset` is
called with different URL formats.
*
[`core-web/libs/portlets/edit-ema/portlet/src/lib/services/guards/edit-ema.guard.spec.ts`](diffhunk://#diff-564087dbe85c3bd9e96481e7adf8b2029561d91b9ce061e8991cb1d10b9b8e31L96-R96):
Updated test cases to reflect the changes in URL sanitization logic.

### Code Cleanup:

*
[`core-web/libs/portlets/edit-ema/portlet/src/lib/store/features/load/withLoad.ts`](diffhunk://#diff-0cbc04b5047422551dde0970e4ea1d0e1fcce3d9d8f4e0b4612fbc30b1658487R84):
Added a comment suggesting the use of `retryWhen()` for potential future
improvements.

---------

Co-authored-by: Kevin Davila <[email protected]>
  • Loading branch information
@KevinDavilaDotCMS @kevindaviladev
KevinDavilaDotCMS and kevindaviladev authored Jan 15, 2025
1 parent 2653b71 commit 90b0c6c
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 35 deletions.
69 changes: 69 additions & 0 deletions ...-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,75 @@ describe('DotEmaShellComponent', () => {
expect(spyStoreLoadPage).toHaveBeenCalledWith(INITIAL_PAGE_PARAMS);
});

describe('Sanitize url when called loadPageAsset', () => {
it('should sanitize when url is index', () => {
const spyloadPageAsset = jest.spyOn(store, 'loadPageAsset');
const spyLocation = jest.spyOn(location, 'go');

const params = {
...INITIAL_PAGE_PARAMS,
url: '/index'
};

overrideRouteSnashot(
activatedRoute,
SNAPSHOT_MOCK({ queryParams: params, data: UVE_CONFIG_MOCK(BASIC_OPTIONS) })
);

spectator.detectChanges();
expect(spyloadPageAsset).toHaveBeenCalledWith({ ...params, url: 'index' });
expect(spyLocation).toHaveBeenCalledWith(
'/?language_id=1&url=index&variantName=DEFAULT&com.dotmarketing.persona.id=modes.persona.no.persona&editorMode=edit'
);
});

it('should sanitize when url is nested', () => {
const spyloadPageAsset = jest.spyOn(store, 'loadPageAsset');

const spyLocation = jest.spyOn(location, 'go');

const params = {
...INITIAL_PAGE_PARAMS,
url: '/some-url/some-nested-url/'
};

overrideRouteSnashot(
activatedRoute,
SNAPSHOT_MOCK({ queryParams: params, data: UVE_CONFIG_MOCK(BASIC_OPTIONS) })
);

spectator.detectChanges();
expect(spyloadPageAsset).toHaveBeenCalledWith({
...params,
url: 'some-url/some-nested-url'
});
expect(spyLocation).toHaveBeenCalledWith(
'/?language_id=1&url=some-url%2Fsome-nested-url&variantName=DEFAULT&com.dotmarketing.persona.id=modes.persona.no.persona&editorMode=edit'
);
});

it('should sanitize when url is nested and ends in index', () => {
const spyloadPageAsset = jest.spyOn(store, 'loadPageAsset');
const spyLocation = jest.spyOn(location, 'go');

const params = {
...INITIAL_PAGE_PARAMS,
url: '/some-url/index'
};

overrideRouteSnashot(
activatedRoute,
SNAPSHOT_MOCK({ queryParams: params, data: UVE_CONFIG_MOCK(BASIC_OPTIONS) })
);

spectator.detectChanges();
expect(spyloadPageAsset).toHaveBeenCalledWith({ ...params, url: 'some-url/' });
expect(spyLocation).toHaveBeenCalledWith(
'/?language_id=1&url=some-url%2F&variantName=DEFAULT&com.dotmarketing.persona.id=modes.persona.no.persona&editorMode=edit'
);
});
});

it('should patch viewParams with empty object when the editorMode is edit', () => {
const patchViewParamsSpy = jest.spyOn(store, 'patchViewParams');
const params = {
Expand Down
4 changes: 4 additions & 0 deletions core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
checkClientHostAccess,
getAllowedPageParams,
getTargetUrl,
sanitizeURL,
shouldNavigate
} from '../utils';

Expand Down Expand Up @@ -209,6 +210,9 @@ export class DotEmaShellComponent implements OnInit {
const params = getAllowedPageParams(queryParams);
const validHost = checkClientHostAccess(params.clientHost, allowedDevURLs);

//Sanitize the url
params.url = sanitizeURL(params.url);

if (!validHost) {
delete params.clientHost;
}
Expand Down
23 changes: 1 addition & 22 deletions core-web/libs/portlets/edit-ema/portlet/src/lib/services/guards/edit-ema.guard.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,28 +93,7 @@ describe('EditEmaGuard', () => {
expect(didEnteredPortlet).toBe(true);
});

it('should navigate to "edit-page" and sanitize url', () => {
const route: ActivatedRouteSnapshot = {
firstChild: {
url: [{ path: 'content' }]
},
queryParams: { url: '/some-url/with-index/index' }
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any;

TestBed.runInInjectionContext(() => editEmaGuard(route, state) as Observable<boolean>);

expect(router.navigate).toHaveBeenCalledWith(['/edit-page/content'], {
queryParams: {
'com.dotmarketing.persona.id': 'modes.persona.no.persona',
language_id: 1,
url: 'some-url/with-index/'
},
replaceUrl: true
});
});

it('should navigate to "edit-page" and sanitize url when the url is "/"', () => {
it('should navigate to "edit-page" with url as "index" when the initial url queryParam is "/"', () => {
const route: ActivatedRouteSnapshot = {
firstChild: {
url: [{ path: 'content' }]
Expand Down
17 changes: 4 additions & 13 deletions core-web/libs/portlets/edit-ema/portlet/src/lib/services/guards/edit-ema.guard.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { inject } from '@angular/core';
import { ActivatedRouteSnapshot, CanActivateFn, Params, Router } from '@angular/router';

import { DEFAULT_PERSONA } from '../../shared/consts';
import { sanitizeURL } from '../../utils';

type EmaQueryParams = {
url: string;
Expand Down Expand Up @@ -44,19 +43,11 @@ function confirmQueryParams(queryParams: Params): {
return acc;
}

// Handle URL parameter special cases
if (key === 'url') {
if (queryParams[key] !== 'index' && queryParams[key].endsWith('/index')) {
acc[key] = sanitizeURL(queryParams[key]);
acc.missing = true;
}

if (queryParams[key] === '/') {
acc[key] = 'index';
acc.missing = true;
if (key === 'url' && queryParams[key] === '/') {
acc[key] = 'index';
acc.missing = true;

return acc;
}
return acc;
}

return acc;
Expand Down
1 change: 1 addition & 0 deletions core-web/libs/portlets/edit-ema/portlet/src/lib/store/features/load/withLoad.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export function withLoad() {
return of(pageAsset);
}

// Maybe we can use retryWhen() instead of this navigate.
const url = vanityUrl.forwardTo.replace('/', '');
router.navigate([], {
queryParamsHandling: 'merge',
Expand Down

0 comments on commit 90b0c6c

Please sign in to comment.