Skip to content

Conversation

@AlexJacksonDS
Copy link
Contributor

Added back a method to figure out an endpoint from the specified endpoint so that it works with the site hosted in a subdirectory.

Copy link
Contributor

@DanBloxham-sw DanBloxham-sw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me.


private static getPathForEndpoint(endpoint: string): string {
const currentPath = window.location.pathname;
const baseUrlParts = endpoint.split('/');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the 'endpoint' (route) was a relative path (I feel like baseUrlParts can be a misleading name)?

const baseUrlParts = endpoint.split('/');
const indexOfBaseUrl = currentPath.indexOf(baseUrlParts[1]);
const path = `${currentPath.substring(0, indexOfBaseUrl)}${endpoint}`;
return path.replace('//', '/');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we have the endpoint without "/" in the beginning to avoid skipping the first element when we split the endpoint and to avoid this replace here? Also, could we make sure it's documented that the endpoints would have to be on the same sub-directory after the base url in config for search to work? (e.g. /dev-prev/GenericItems would not work with '/AllGenericItems' as endpoint on UAT, /TrackingSystem can't call /LearningPortal etc)

Copy link
Contributor

@stellake stellake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 👍

@AlexJacksonDS AlexJacksonDS merged commit 0573797 into master Jun 24, 2021
@AlexJacksonDS AlexJacksonDS deleted the HEEDLS-415-admin-page-pagination branch June 24, 2021 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants