fix(settings routing): handle trailing slashes in base paths (#10055)

Adjusted URL construction to properly handle trailing slashes in base
paths, ensuring consistent matching logic. Added logic for setting the
hotkey scope when navigating to the domain settings path.
This commit is contained in:
Antoine Moreaux
2025-02-06 17:07:19 +01:00
committed by GitHub
parent 5b79ac771c
commit a85c4f263a
10 changed files with 106 additions and 51 deletions

View File

@ -1,39 +1,74 @@
import { MemoryRouter } from 'react-router-dom';
import { renderHook } from '@testing-library/react';
import { useIsMatchingLocation } from '../useIsMatchingLocation';
import { MemoryRouter } from 'react-router-dom';
import { AppBasePath } from '@/types/AppBasePath';
import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation';
const Wrapper = ({ children }: { children: React.ReactNode }) => (
<MemoryRouter
initialEntries={['/one', '/two', { pathname: '/three' }]}
initialIndex={1}
>
{children}
</MemoryRouter>
);
const Wrapper =
(initialIndex = 0) =>
({ children }: { children: React.ReactNode }) => (
<MemoryRouter
initialEntries={['/example', '/other', `${AppBasePath.Settings}/example`]}
initialIndex={initialIndex}
>
{children}
</MemoryRouter>
);
describe('useIsMatchingLocation', () => {
it('should return true for a matching location', () => {
const { result } = renderHook(
() => {
const checkMatchingLocation = useIsMatchingLocation();
return checkMatchingLocation('/two');
},
{ wrapper: Wrapper },
);
it('returns true when paths match with no basePath', () => {
const { result } = renderHook(() => useIsMatchingLocation(), {
wrapper: Wrapper(),
});
expect(result.current).toBe(true);
expect(result.current.isMatchingLocation('/example')).toBe(true);
});
it('should return false for a non-matching location', () => {
const { result } = renderHook(
() => {
const checkMatchingLocation = useIsMatchingLocation();
return checkMatchingLocation('/four');
},
{ wrapper: Wrapper },
);
it('returns false when paths do not match with no basePath', () => {
const { result } = renderHook(() => useIsMatchingLocation(), {
wrapper: Wrapper(),
});
expect(result.current).toBe(false);
expect(result.current.isMatchingLocation('/non-match')).toBe(false);
});
it('returns true when paths match with basePath', () => {
const { result } = renderHook(() => useIsMatchingLocation(), {
wrapper: Wrapper(2),
});
expect(
result.current.isMatchingLocation('example', AppBasePath.Settings),
).toBe(true);
});
it('returns false when paths do not match with basePath', () => {
const { result } = renderHook(() => useIsMatchingLocation(), {
wrapper: Wrapper(),
});
expect(
result.current.isMatchingLocation('non-match', AppBasePath.Settings),
).toBe(false);
});
it('handles trailing slashes in basePath correctly', () => {
const { result } = renderHook(() => useIsMatchingLocation(), {
wrapper: Wrapper(2),
});
expect(
result.current.isMatchingLocation(
'example',
(AppBasePath.Settings + '/') as AppBasePath,
),
).toBe(true);
});
it('handles without basePath correctly', () => {
const { result } = renderHook(() => useIsMatchingLocation(), {
wrapper: Wrapper(),
});
expect(result.current.isMatchingLocation('example')).toBe(true);
});
});

View File

@ -30,9 +30,9 @@ jest.mock('~/hooks/useIsMatchingLocation');
const mockUseIsMatchingLocation = jest.mocked(useIsMatchingLocation);
const setupMockIsMatchingLocation = (pathname: string) => {
mockUseIsMatchingLocation.mockReturnValueOnce(
(path: string) => path === pathname,
);
mockUseIsMatchingLocation.mockReturnValueOnce({
isMatchingLocation: (path: string) => path === pathname,
});
};
jest.mock('@/auth/hooks/useIsLogged');

View File

@ -1,19 +1,32 @@
import { useCallback } from 'react';
import { matchPath, useLocation } from 'react-router-dom';
import { AppBasePath } from '@/types/AppBasePath';
import { isNonEmptyString } from '@sniptt/guards';
import { isDefined } from 'twenty-shared';
export const useIsMatchingLocation = () => {
const location = useLocation();
return useCallback(
(path: string, basePath?: AppBasePath) => {
const constructedPath = basePath
? (new URL(basePath + path, document.location.origin).pathname ?? '')
: path;
const addTrailingSlash = (path: string) =>
path.endsWith('/') ? path : path + '/';
return !!matchPath(constructedPath, location.pathname);
},
[location.pathname],
);
const getConstructedPath = (path: string, basePath?: AppBasePath) => {
if (!isNonEmptyString(basePath)) return path;
return addTrailingSlash(basePath) + path;
};
const isMatchingLocation = (path: string, basePath?: AppBasePath) => {
const match = matchPath(
getConstructedPath(path, basePath),
location.pathname,
);
const isMatching = isDefined(match);
return isMatching;
};
return {
isMatchingLocation,
};
};

View File

@ -8,7 +8,7 @@ import { OnboardingStatus } from '~/generated/graphql';
import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation';
export const usePageChangeEffectNavigateLocation = () => {
const isMatchingLocation = useIsMatchingLocation();
const { isMatchingLocation } = useIsMatchingLocation();
const isLoggedIn = useIsLogged();
const onboardingStatus = useOnboardingStatus();
const isWorkspaceSuspended = useIsWorkspaceActivationStatusSuspended();

View File

@ -24,7 +24,7 @@ export const useApolloFactory = (options: Partial<Options<any>> = {}) => {
const [isDebugMode] = useRecoilState(isDebugModeState);
const navigate = useNavigate();
const isMatchingLocation = useIsMatchingLocation();
const { isMatchingLocation } = useIsMatchingLocation();
const [tokenPair, setTokenPair] = useRecoilState(tokenPairState);
const [currentWorkspace, setCurrentWorkspace] = useRecoilState(
currentWorkspaceState,

View File

@ -29,7 +29,7 @@ import { usePageChangeEffectNavigateLocation } from '~/hooks/usePageChangeEffect
// - moved usePageChangeEffectNavigateLocation into dedicated hook
export const PageChangeEffect = () => {
const navigate = useNavigate();
const isMatchingLocation = useIsMatchingLocation();
const { isMatchingLocation } = useIsMatchingLocation();
const [previousLocation, setPreviousLocation] = useState('');
@ -140,6 +140,13 @@ export const PageChangeEffect = () => {
});
break;
}
case isMatchingLocation(SettingsPath.Domain, AppBasePath.Settings): {
setHotkeyScope(PageHotkeyScope.Settings, {
goto: false,
keyboardShortcutMenu: true,
});
break;
}
case isMatchingLocation(
SettingsPath.WorkspaceMembersPage,
AppBasePath.Settings,

View File

@ -24,7 +24,7 @@ export const useSignInUp = (form: UseFormReturn<Form>) => {
const [signInUpStep, setSignInUpStep] = useRecoilState(signInUpStepState);
const [signInUpMode, setSignInUpMode] = useRecoilState(signInUpModeState);
const isMatchingLocation = useIsMatchingLocation();
const { isMatchingLocation } = useIsMatchingLocation();
const workspaceInviteHash = useParams().workspaceInviteHash;
const [searchParams] = useSearchParams();

View File

@ -34,9 +34,9 @@ jest.mock('~/hooks/useIsMatchingLocation');
const mockUseIsMatchingLocation = jest.mocked(useIsMatchingLocation);
const setupMockIsMatchingLocation = (pathname: string) => {
mockUseIsMatchingLocation.mockReturnValueOnce(
(path: string) => path === pathname,
);
mockUseIsMatchingLocation.mockReturnValueOnce({
isMatchingLocation: (path: string) => path === pathname,
});
};
const getResult = (isDefaultLayoutAuthModalVisible = true) =>

View File

@ -11,7 +11,7 @@ import { OnboardingStatus, SubscriptionStatus } from '~/generated/graphql';
import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation';
export const useShowAuthModal = () => {
const isMatchingLocation = useIsMatchingLocation();
const { isMatchingLocation } = useIsMatchingLocation();
const isLoggedIn = useIsLogged();
const onboardingStatus = useOnboardingStatus();
const subscriptionStatus = useSubscriptionStatus();

View File

@ -10,7 +10,7 @@ import { UserOrMetadataLoader } from '~/loading/components/UserOrMetadataLoader'
export const UserProvider = ({ children }: React.PropsWithChildren) => {
const isCurrentUserLoaded = useRecoilValue(isCurrentUserLoadedState);
const isMatchingLocation = useIsMatchingLocation();
const { isMatchingLocation } = useIsMatchingLocation();
const dateTimeFormat = useRecoilValue(dateTimeFormatState);