Skip to content

Commit

Permalink
fix(uve): Added trailing slash when the url ends with /index (#31093)
Browse files Browse the repository at this point in the history
PR Changes:



https://github.com/user-attachments/assets/2329986e-5b34-4fc1-bc6d-4c7224199399

Algo this PR solves a VanityURL issue inside UVE:
Issue before PR:



https://github.com/user-attachments/assets/3bd2978d-9e30-4da0-af3c-7998c2b4a3ab

Fix:



https://github.com/user-attachments/assets/419b407a-9359-4af0-b380-1d76dfb5430d



This pull request includes several changes to the
`core-web/libs/portlets/edit-ema/portlet` module, focusing on URL
sanitization and handling vanity URLs. The most important changes
include modifications to the `DotPageApiService`, updates to the
`sanitizeURL` utility function, and enhancements to the `withEditor`
store feature.

### URL Sanitization and Handling:

*
[`core-web/libs/portlets/edit-ema/portlet/src/lib/services/dot-page-api.service.ts`](diffhunk://#diff-9acb0e7dc2619395c49047164381778d2b6d6c41f58b30aa3cd6a798044007c6L107-R108):
Updated the URL sanitization logic to only remove leading slashes,
instead of both leading and trailing slashes.
*
[`core-web/libs/portlets/edit-ema/portlet/src/lib/utils/index.ts`](diffhunk://#diff-2fd4d225bb33327848da73766ec5efbe037e3aed1a1e51a50c19f6a2be8cdb80L201-R202):
Modified the `sanitizeURL` function to replace 'index' or '/index' at
the end of the URL with a single slash, and to remove both starting and
trailing slashes.

### Test Adjustments:

*
[`core-web/libs/portlets/edit-ema/portlet/src/lib/services/dot-page-api.service.spec.ts`](diffhunk://#diff-1dad8c8d685da624eaecb75a9607377dfa7168120d182df479ffceaed1a87858L86-R89):
Updated tests to reflect the new URL sanitization logic by removing
trailing slashes only.
[[1]](diffhunk://#diff-1dad8c8d685da624eaecb75a9607377dfa7168120d182df479ffceaed1a87858L86-R89)
[[2]](diffhunk://#diff-1dad8c8d685da624eaecb75a9607377dfa7168120d182df479ffceaed1a87858L133-R133)
*
[`core-web/libs/portlets/edit-ema/portlet/src/lib/utils/utils.spec.ts`](diffhunk://#diff-6b95047786d13d69ebdcb642c6d382bc35784a2ada53f95d1908119ab08dfb91L323-R327):
Adjusted tests for the `sanitizeURL` function to validate the new
behavior of handling 'index' and slashes.
[[1]](diffhunk://#diff-6b95047786d13d69ebdcb642c6d382bc35784a2ada53f95d1908119ab08dfb91L323-R327)
[[2]](diffhunk://#diff-6b95047786d13d69ebdcb642c6d382bc35784a2ada53f95d1908119ab08dfb91R339-R342)

### Vanity URL Handling:

*
[`core-web/libs/portlets/edit-ema/portlet/src/lib/store/features/editor/withEditor.ts`](diffhunk://#diff-86e692578757ed7f4f6cba5d0aeb07641312f3b17885825d1a45987153ae87f0R203-R205):
Enhanced the `withEditor` store feature to properly handle vanity URLs
by prioritizing the vanity URL over the page URI.
*
[`core-web/libs/portlets/edit-ema/portlet/src/lib/store/features/editor/withEditor.spec.ts`](diffhunk://#diff-48915538475425e5656151bb2a73df7a99b4c80c2817c085f96352ef3252d5cbR697-R712):
Added a new test to ensure the correct URL is used when the page has a
vanity URL.

These changes ensure more consistent URL handling across the application
and improve the robustness of the URL-related logic.

---------

Co-authored-by: Kevin Davila <[email protected]>
  • Loading branch information
KevinDavilaDotCMS and kevindaviladev authored Jan 10, 2025
1 parent 83e3277 commit dfbe70e
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ describe('DotPageApiService', () => {
);
});

it('should remove trailing and leading slashes in the url', () => {
it('should remove all starting and trailing slashes in the url', () => {
spectator.service
.get({
url: '///test-url///',
url: '///test-url',
language_id: 'en',
'com.dotmarketing.persona.id': 'modes.persona.no.persona'
})
Expand All @@ -98,6 +98,21 @@ describe('DotPageApiService', () => {
);
});

it('should mantain final trailing slash in the url', () => {
spectator.service
.get({
url: '///my-folder///',
language_id: 'en',
'com.dotmarketing.persona.id': 'modes.persona.no.persona'
})
.subscribe();

spectator.expectOne(
'/api/v1/page/render/my-folder/?language_id=en&com.dotmarketing.persona.id=modes.persona.no.persona&variantName=DEFAULT&depth=0&mode=EDIT_MODE',
HttpMethod.GET
);
});

it('should get the page using graphql', () => {
const query = 'query { ... }';
spectator.service.getGraphQLPage(query).subscribe();
Expand Down Expand Up @@ -130,7 +145,7 @@ describe('DotPageApiService', () => {

describe('getClientPage', () => {
const baseParams = {
url: '///test-url///',
url: '///test-url',
language_id: 'en',
'com.dotmarketing.persona.id': 'modes.persona.no.persona'
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
import { PAGE_MODE } from '../shared/enums';
import { DotPage, DotPageAssetParams, SavePagePayload } from '../shared/models';
import { ClientRequestProps } from '../store/features/client/withClient';
import { createPageApiUrlWithQueryParams } from '../utils';
import { cleanPageURL, createPageApiUrlWithQueryParams } from '../utils';

export interface DotPageApiResponse {
page: DotPage;
Expand Down Expand Up @@ -104,7 +104,8 @@ export class DotPageApiService {
experimentId,
publishDate
} = params;
const url = params.url.replace(/^\/+|\/+$/g, '');

const url = cleanPageURL(params.url);

const pageType = clientHost ? 'json' : 'render';
const isPreview = editorMode === UVE_MODE.PREVIEW;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ describe('EditEmaGuard', () => {
queryParams: {
'com.dotmarketing.persona.id': 'modes.persona.no.persona',
language_id: 1,
url: 'some-url/with-index'
url: 'some-url/with-index/'
},
replaceUrl: true
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,22 @@ describe('withEditor', () => {

expect(store.$iframeURL()).toContain('about:blank');
});

it('should contain the right url when the page is a vanity url ', () => {
patchState(store, {
pageAPIResponse: {
...MOCK_RESPONSE_HEADLESS,
vanityUrl: {
...MOCK_RESPONSE_HEADLESS.vanityUrl,
url: 'first'
}
}
});

expect(store.$iframeURL()).toBe(
'http://localhost:3000/first?language_id=1&com.dotmarketing.persona.id=dot%3Apersona&variantName=DEFAULT&clientHost=http%3A%2F%2Flocalhost%3A3000'
);
});
});

describe('$editorProps', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,9 @@ export function withEditor() {
}),
$iframeURL: computed<string>(() => {
const page = store.pageAPIResponse().page;
const vanityURL = store.pageAPIResponse().vanityUrl?.url;
const url = buildIframeURL({
pageURI: page?.pageURI,
pageURI: vanityURL ?? page?.pageURI,
params: store.pageParams(),
isTraditionalPage: untracked(() => store.isTraditionalPage())
});
Expand Down
18 changes: 16 additions & 2 deletions core-web/libs/portlets/edit-ema/portlet/src/lib/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,8 @@ function insertPositionedContentletInContainer(payload: ActionPayload): {
*/
export function sanitizeURL(url?: string): string {
return url
?.replace(/(^\/)|(\/$)/g, '') // Remove slashes from the beginning and end of the url
.replace(/\/index$/, ''); // Remove index from the end of the url
?.replace(/^\/+|\/+$/g, '') // Remove starting and trailing slashes
?.replace(/(?:\/)?index\/?$/, '/'); // Replace 'index' or '/index' at the end with a single slash // Replace 'index' with a slash if it's not preceded by a slash
}

/**
Expand Down Expand Up @@ -675,3 +675,17 @@ export const getWrapperMeasures = (
height: `${Math.max(Number(device?.cssHeight), Number(device?.cssWidth))}${unit}`
};
};

/**
* Cleans and normalizes a page URL by:
* 1. Removing leading slashes while preserving trailing slash if present
* 2. Converting multiple consecutive slashes at end into single slashes
*
* @param {string} url - The URL to clean
* @returns {string} The cleaned URL with normalized slashes
*/
export const cleanPageURL = (url: string) => {
return url
.replace(/^\/*(.*?)(\/+)?$/, '$1$2') // Capture content and optional trailing slash
.replace(/\/+/g, '/'); // Clean up any remaining multiple slashes
};
Original file line number Diff line number Diff line change
Expand Up @@ -320,11 +320,11 @@ describe('utils functions', () => {
});

it('should remove the index if a nested path', () => {
expect(sanitizeURL('i-have-the-high-ground/index')).toEqual('i-have-the-high-ground');
expect(sanitizeURL('i-have-the-high-ground/index')).toEqual('i-have-the-high-ground/');
});

it('should remove the index if a nested path with slash', () => {
expect(sanitizeURL('no-index-please/index/')).toEqual('no-index-please');
expect(sanitizeURL('no-index-please/index/')).toEqual('no-index-please/');
});

it('should leave as it is for valid url', () => {
Expand All @@ -336,6 +336,10 @@ describe('utils functions', () => {
it('should leave as it is for a nested valid url', () => {
expect(sanitizeURL('hello-there/general-kenobi')).toEqual('hello-there/general-kenobi');
});

it('should remove index from the end of the url but keep the slash if is a nested path', () => {
expect(sanitizeURL('my-nested-path/index/')).toEqual('my-nested-path/');
});
});

describe('personalization', () => {
Expand Down

0 comments on commit dfbe70e

Please sign in to comment.