Skip to content

Commit

Permalink
Fixed: Fix internal Final button link (v2) (#1063)
Browse files Browse the repository at this point in the history
* fix: Fix internal Final button link, which wouldn't cause an experiment to reload as it was already on the same location, by introducing an InternalRedirect

* fix: Fix relative url button link test & add test for button without link
  • Loading branch information
drikusroor authored Jun 5, 2024
1 parent f6183fd commit 2fc2bc6
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 2 deletions.
3 changes: 3 additions & 0 deletions frontend/src/components/App/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import Profile from "../Profile/Profile";
import Reload from "../Reload/Reload";
import StoreProfile from "../StoreProfile/StoreProfile";
import useDisableRightClickOnTouchDevices from "../../hooks/useDisableRightClickOnTouchDevices";
import { InternalRedirect } from "../InternalRedirect/InternalRedirect";


// App is the root component of our application
Expand Down Expand Up @@ -75,6 +76,8 @@ const App = () => {
<Profile slug={EXPERIMENT_SLUG} />
</Route>

<Route path={URLS.internalRedirect} component={InternalRedirect} />

{/* Experiment Collection */}
<Route path={URLS.experimentCollection} component={ExperimentCollection} />

Expand Down
19 changes: 18 additions & 1 deletion frontend/src/components/Final/Final.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ session="session-id"

const el = screen.getByTestId('button-link');
expect(el).to.exist;
expect(el.getAttribute('href')).toBe('/aml');
expect(el.getAttribute('href')).toBe('/redirect/aml');
});

it('Uses an anchor tag to navigate when button link is absolute', () => {
Expand All @@ -162,4 +162,21 @@ session="session-id"
expect(el).to.exist;
expect(el.getAttribute('href')).toBe('https://example.com');
});

it('Calls onNext when there is no button link and the user clicks the button', async () => {
const onNextMock = vi.fn();
render(
<BrowserRouter>
<Final
button={{ text: 'Next' }}
onNext={onNextMock}
/>
</BrowserRouter>
);

fireEvent.click(screen.getByTestId('button'));
await waitFor(() => {
expect(onNextMock).toHaveBeenCalled();
});
});
});
4 changes: 3 additions & 1 deletion frontend/src/components/Final/FinalButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@ const FinalButton: React.FC<FinalButtonProps> = ({ button, onNext }) => {

// If the button has a link, it will render a Link component if the link is a relative URL
if (isRelativeUrl(button.link)) {
const url = `/redirect${button.link}`

return (
<Link data-testid="button-link" className='btn btn-primary btn-lg' to={button.link}>
<Link data-testid="button-link" className='btn btn-primary btn-lg' to={url}>
{button.text}
</Link>
)
Expand Down
13 changes: 13 additions & 0 deletions frontend/src/components/InternalRedirect/InternalRedirect.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import React from 'react';
import { Redirect, RouteComponentProps } from 'react-router-dom';

// this component is a route, so it will receive the route props
interface InternalRedirectProps extends RouteComponentProps {}

export const InternalRedirect: React.FC<InternalRedirectProps> = (props) => {
const { path } = props.match.params as { path: string };
const { search } = props.location;

// Redirect to the experiment path
return <Redirect to={`/${path}${search}`} />;
}
1 change: 1 addition & 0 deletions frontend/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export const URLS = {
experiment: "/:slug",
experimentCollectionAbout: "/collection/:slug/about",
experimentCollection: "/collection/:slug",
internalRedirect: "/redirect/:path",
reloadParticipant: "/participant/reload/:id/:hash",
theme: "/theme/:id",
AMLHome:
Expand Down

0 comments on commit 2fc2bc6

Please sign in to comment.