Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove sessionId from URLs #2485

Merged
merged 3 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion editor.planx.uk/src/pages/Preview/ResumePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { ApplicationPath, SendEmailPayload } from "types";
import Input from "ui/Input";
import InputLabel from "ui/InputLabel";
import InputRow from "ui/InputRow";
import { removeSessionIdSearchParamWithoutReloading } from "utils";
import { object, string } from "yup";

import ReconciliationPage from "./ReconciliationPage";
Expand Down Expand Up @@ -215,7 +216,14 @@ const ResumePage: React.FC = () => {
getInitialEmailValue(route.url.query.email),
);
const [paymentRequest, setPaymentRequest] = useState<MinPaymentRequest>();
const sessionId = useCurrentRoute().url.query.sessionId;

// Read the sessionId from the url to validate against
const sessionId = route.url.query.sessionId;

// As the sessionId has been extracted it can now be removed to avoid
// unnecessarily exposing it
removeSessionIdSearchParamWithoutReloading();

const [reconciliationResponse, setReconciliationResponse] =
useState<ReconciliationResponse>();

Expand Down
4 changes: 2 additions & 2 deletions editor.planx.uk/src/pages/Preview/SaveAndReturn.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe("Save and Return component", () => {
expect(results).toHaveNoViolations();
});

it("stores the sessionId as part of the URL once an email has been submitted", async () => {
it("does not store the sessionId as part of the URL once an email has been submitted", async () => {
const children = <Button>Testing 123</Button>;
const { user } = setup(<SaveAndReturn children={children}></SaveAndReturn>);

Expand All @@ -89,7 +89,7 @@ describe("Save and Return component", () => {
expect(screen.getByText("Testing 123")).toBeInTheDocument();
});

expect(window.location.href).toContain(`sessionId=${sessionId}`);
Copy link
Member

@jessicamcinchak jessicamcinchak Nov 27, 2023

Choose a reason for hiding this comment

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

nit: would it perhaps be more meaningful to flip this test to check not.toContain rather than removing it all together?

expect(window.location.href).not.toContain(`sessionId=${sessionId}`);
});
});

Expand Down
10 changes: 0 additions & 10 deletions editor.planx.uk/src/pages/Preview/SaveAndReturn.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,20 +84,10 @@ const SaveAndReturn: React.FC<{ children: React.ReactNode }> = ({
children,
}) => {
const isEmailCaptured = Boolean(useStore((state) => state.saveToEmail));
const sessionId = useStore((state) => state.sessionId);
const isContentPage = useCurrentRoute()?.data?.isContentPage;

// Setting the URL search param "sessionId" will route the user to ApplicationPath.Resume
// Without this the user will need to click the magic link in their email after a refresh
const allowResumeOnBrowserRefresh = () => {
const url = new URL(window.location.href);
url.searchParams.set("sessionId", sessionId);
window.history.pushState({}, document.title, url);
};

const handleSubmit = (email: string) => {
useStore.setState({ saveToEmail: email });
allowResumeOnBrowserRefresh();
};

return (
Expand Down
6 changes: 6 additions & 0 deletions editor.planx.uk/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,9 @@ export const removeSessionIdSearchParam = () => {
window.history.pushState({}, document.title, currentURL);
window.location.reload();
};

export const removeSessionIdSearchParamWithoutReloading = () => {
const currentURL = new URL(window.location.href);
currentURL.searchParams.delete("sessionId");
window.history.replaceState({}, document.title, currentURL);
};
Loading