Skip to content

Commit

Permalink
remove autologin from registration (#137)
Browse files Browse the repository at this point in the history
  • Loading branch information
hannah-whelan authored Apr 24, 2024
1 parent 253b3d9 commit 1639075
Show file tree
Hide file tree
Showing 22 changed files with 1,840 additions and 99 deletions.
1,725 changes: 1,725 additions & 0 deletions public/assets/checkemail.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion src/IsaacAppTypes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ export type Action =
| { type: ACTION_TYPE.TOPIC_RESPONSE_SUCCESS; topic: ApiTypes.IsaacTopicSummaryPageDTO }
| { type: ACTION_TYPE.TOPIC_RESPONSE_FAILURE }
| { type: ACTION_TYPE.ACCOUNT_UPGRADE_SEND_REQUEST }
| { type: ACTION_TYPE.ACCOUNT_UPGRADE_SEND_RESPONSE_SUCCESS; user: Immutable<ApiTypes.RegisteredUserDTO> }
| { type: ACTION_TYPE.ACCOUNT_UPGRADE_SEND_RESPONSE_SUCCESS }
| { type: ACTION_TYPE.ACCOUNT_UPGRADE_SEND_RESPONSE_FAILURE; errorMessage: string }
| { type: ACTION_TYPE.CONTACT_FORM_SEND_REQUEST }
| { type: ACTION_TYPE.CONTACT_FORM_SEND_RESPONSE_SUCCESS }
Expand Down
12 changes: 5 additions & 7 deletions src/app/components/hooks/useRegistration.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useState } from "react";
import { FIRST_LOGIN_STATE, KEY, allowedDomain, persistence, validateForm } from "../../services";
import { useAppDispatch, registerUser, upgradeAccount, requestCurrentUser } from "../../state";
import { allowedDomain, validateForm } from "../../services";
import { useAppDispatch, registerUser, upgradeAccount } from "../../state";
import { BooleanNotation, DisplaySettings, UserEmailPreferences, ValidationUser } from "../../../IsaacAppTypes";
import { UserContext } from "../../../IsaacApiTypes";
import ReactGA from "react-ga4";
Expand Down Expand Up @@ -67,16 +67,14 @@ const useRegistration = ({ isTeacher }: RegistrationOptions) => {
action: "registration",
label: "Create Account (SEGUE)",
});
// send email for account upgrade request
await dispatch(upgradeAccount({ verificationDetails, otherInformation }));
await dispatch(requestCurrentUser());
// send email for account upgrade request - email definitely exists as we've already validated it
const userEmail = registrationUser.email!;
await dispatch(upgradeAccount({ verificationDetails, userEmail, otherInformation }));
}
};

const handleStudentRegistration = () => {
if (isValidForm) {
persistence.session.save(KEY.FIRST_LOGIN, FIRST_LOGIN_STATE.FIRST_LOGIN);

dispatch(registerUser(newUser, userPreferencesToUpdate, userContexts, recaptchaToken));
ReactGA.event({
category: "user",
Expand Down
2 changes: 2 additions & 0 deletions src/app/components/navigation/IsaacApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ import { ExternalRedirect } from "../handlers/ExternalRedirect";
import { StudentRegistration } from "../pages/StudentRegistration";
import { TeacherRegistration } from "../pages/TeacherRegistration";
import { WorkbookDownload } from "../pages/WorkbookDownload";
import { RegistrationSuccess } from "../pages/RegistrationSuccess";

const ContentEmails = lazy(() => import("../pages/ContentEmails"));
const MyProgress = lazy(() => import("../pages/MyProgress"));
Expand Down Expand Up @@ -328,6 +329,7 @@ export const IsaacApp = () => {
<TrackedRoute exact path="/register" component={Registration} />
<TrackedRoute exact path="/register/student" component={StudentRegistration} />
<TrackedRoute exact path="/register/teacher" component={TeacherRegistration} />
<TrackedRoute exact path="/register/success" component={RegistrationSuccess} />
<TrackedRoute exact path="/auth/:provider/callback" component={ProviderCallbackHandler} />
<TrackedRoute exact path="/resetpassword/:token" component={ResetPasswordHandler} />
<TrackedRoute exact path="/verifyemail" component={EmailAlterHandler} />
Expand Down
2 changes: 1 addition & 1 deletion src/app/components/pages/LogIn.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const useLoginLogic = () => {

const signUp = (event: React.MouseEvent) => {
event.preventDefault();
history.push("/register", { email: email, password: password });
history.push("/register", { email: email });
};

const attemptLogIn = () => {
Expand Down
2 changes: 0 additions & 2 deletions src/app/components/pages/MyAccount.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ const stateToProps = (state: AppState, props: any) => {
errorMessage: state?.error ?? null,
userAuthSettings: state?.userAuthSettings ?? null,
userPreferences: state?.userPreferences ?? null,
firstLogin: (history?.location?.state as { firstLogin: any } | undefined)?.firstLogin,
hashAnchor: hash?.slice(1) ?? null,
authToken: (searchParams?.authToken as string) ?? null,
userOfInterest: (searchParams?.userId as string) ?? null,
Expand All @@ -90,7 +89,6 @@ interface AccountPageProps {
userAuthSettings: UserAuthenticationSettingsDTO | null;
getChosenUserAuthSettings: (userid: number) => void;
userPreferences: UserPreferencesDTO | null;
firstLogin: boolean;
hashAnchor: string | null;
authToken: string | null;
userOfInterest: string | null;
Expand Down
5 changes: 3 additions & 2 deletions src/app/components/pages/Registration.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ import {
UncontrolledTooltip,
} from "reactstrap";
import { TitleAndBreadcrumb } from "../elements/TitleAndBreadcrumb";
import { Redirect } from "react-router";
import { Redirect, useLocation } from "react-router";
import { MetaDescription } from "../elements/MetaDescription";
import { Role } from "../../../IsaacApiTypes";

export const Registration = () => {
const location = useLocation();
const user = useAppSelector(selectors.user.orNull);

const [role, setRole] = useState<Role>();
Expand All @@ -42,7 +43,7 @@ export const Registration = () => {
}

if (redirectTo) {
return <Redirect to={redirectTo} />;
return <Redirect to={{ pathname: redirectTo, state: location.state }} />;
}

const metaDescriptionCS =
Expand Down
31 changes: 31 additions & 0 deletions src/app/components/pages/RegistrationSuccess.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import React from "react";
import { Card, CardBody, Container } from "reactstrap";
import { TitleAndBreadcrumb } from "../elements/TitleAndBreadcrumb";
import { REGISTER_CRUMB } from "../../services";
import { selectors, useAppSelector } from "../../state";
import { Redirect } from "react-router";

export const RegistrationSuccess = () => {
const user = useAppSelector(selectors.user.orNull);

if (user?.loggedIn) {
return <Redirect to="/" />;
}

return (
<Container id="registration-success" className="mb-5">
<TitleAndBreadcrumb
currentPageTitle="Thanks for signing up"
intermediateCrumbs={[REGISTER_CRUMB]}
className="mb-4"
/>
<Card>
<CardBody className="text-center">
<img src="/assets/checkemail.svg" alt="check email" className="w-20 mb-4" style={{ maxHeight: "150px" }} />
<h3>Thank you for providing the required account information.</h3>
<p> An email has been sent to your provided email address to complete the registration process.</p>
</CardBody>
</Card>
</Container>
);
};
8 changes: 5 additions & 3 deletions src/app/components/pages/StudentRegistration.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Card, CardBody, CardTitle, Col, Container, Form, FormFeedback, Row } fr
import { BooleanNotation, DisplaySettings, UserEmailPreferences, ValidationUser } from "../../../IsaacAppTypes";
import { loadZxcvbnIfNotPresent, REGISTER_CRUMB, validateForm } from "../../services";
import { TitleAndBreadcrumb } from "../elements/TitleAndBreadcrumb";
import { Redirect } from "react-router";
import { Redirect, useLocation } from "react-router";
import { MetaDescription } from "../elements/MetaDescription";
import { SchoolInput } from "../elements/inputs/SchoolInput";
import { UserContext } from "../../../IsaacApiTypes";
Expand All @@ -21,8 +21,9 @@ import { RegistrationSubmit } from "../elements/inputs/RegistrationSubmit";
import ReCAPTCHA from "react-google-recaptcha";
import { Recaptcha } from "../elements/inputs/Recaptcha";

// TODO: useLocation hook to retrieve email/password when upgrading react router to v6+
export const StudentRegistration = () => {
const location = useLocation();
const { email } = (location.state || {}) as { email?: string };
const user = useAppSelector(selectors.user.orNull);
const errorMessage = useAppSelector(selectors.error.general);
const { register, attemptedSignUp } = useRegistration({ isTeacher: false });
Expand All @@ -37,7 +38,7 @@ export const StudentRegistration = () => {
// Inputs which trigger re-render
const [registrationUser, setRegistrationUser] = useState<Immutable<ValidationUser>>({
...user,
email: undefined,
email: email,
dateOfBirth: undefined,
password: null,
familyName: undefined,
Expand Down Expand Up @@ -130,6 +131,7 @@ export const StudentRegistration = () => {
userToUpdate={registrationUser}
setUserToUpdate={setRegistrationUser}
submissionAttempted={attemptedSignUp}
emailDefault={email}
/>
</Col>
<Col md={6}>
Expand Down
8 changes: 5 additions & 3 deletions src/app/components/pages/TeacherRegistration.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Button, Card, CardBody, CardTitle, Col, Container, Form, FormFeedback,
import { BooleanNotation, DisplaySettings, UserEmailPreferences, ValidationUser } from "../../../IsaacAppTypes";
import { loadZxcvbnIfNotPresent, REGISTER_CRUMB, validateForm } from "../../services";
import { TitleAndBreadcrumb } from "../elements/TitleAndBreadcrumb";
import { Redirect } from "react-router";
import { Redirect, useLocation } from "react-router";
import { MetaDescription } from "../elements/MetaDescription";
import { SchoolInput } from "../elements/inputs/SchoolInput";
import { UserContext } from "../../../IsaacApiTypes";
Expand Down Expand Up @@ -126,8 +126,9 @@ const TeacherRegistrationTerms = ({ acceptConditions }: { acceptConditions: () =
);
};

// TODO: useLocation hook to retrieve email/password when upgrading react router to v6+
const TeacherRegistrationForm = () => {
const location = useLocation();
const { email } = (location.state || {}) as { email?: string };
const user = useAppSelector(selectors.user.orNull);
const errorMessage = useAppSelector(selectors.error.general);
const { register, attemptedSignUp } = useRegistration({ isTeacher: true });
Expand All @@ -144,7 +145,7 @@ const TeacherRegistrationForm = () => {
// Inputs which trigger re-render
const [registrationUser, setRegistrationUser] = useState<Immutable<ValidationUser>>({
...user,
email: undefined,
email: email,
dateOfBirth: undefined,
password: null,
familyName: undefined,
Expand Down Expand Up @@ -240,6 +241,7 @@ const TeacherRegistrationForm = () => {
setUserToUpdate={setRegistrationUser}
submissionAttempted={attemptedSignUp}
teacherRegistration={true}
emailDefault={email}
/>
</Col>
<Col md={6}>
Expand Down
13 changes: 9 additions & 4 deletions src/app/components/pages/TeacherRequest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { useEffect, useState } from "react";
import {
AppState,
isaacApi,
requestCurrentUser,
requestEmailVerification,
selectors,
upgradeAccount,
Expand Down Expand Up @@ -128,14 +129,18 @@ export const TeacherRequest = () => {
</Col>
</Row>
) : (
!isTeacherPending(user) && (
!isTeacherPending(user) &&
user?.loggedIn && (
<Form
name="contact"
onSubmit={(e) => {
e.preventDefault();
if (verificationDetails) {
dispatch(upgradeAccount({ verificationDetails, otherInformation }));
setMessageSent(true);
if (verificationDetails && user?.email && isValidEmail) {
const userEmail = user.email;
dispatch(upgradeAccount({ verificationDetails, userEmail, otherInformation })).then(() => {
setMessageSent(true);
dispatch(requestCurrentUser());
});
}
}}
>
Expand Down
1 change: 1 addition & 0 deletions src/app/services/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export const api = {

upgradeAccount: (params: {
verificationDetails: string;
userEmail: string;
otherInformation?: string;
}): AxiosPromise<ApiTypes.RegisteredUserDTO> => {
return endpoint.post(`/users/request_role_change`, params);
Expand Down
12 changes: 0 additions & 12 deletions src/app/services/firstLogin.ts

This file was deleted.

1 change: 0 additions & 1 deletion src/app/services/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ export * from "./tagsAbstract";
export * from "./tagsCS";
export * from "./tags";
export * from "./questions";
export * from "./firstLogin";
export * from "./notificationChecker";
export * from "./passwordStrength";
export * from "./searchResults";
Expand Down
1 change: 0 additions & 1 deletion src/app/services/localStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
export enum KEY {
AFTER_AUTH_PATH = "afterAuthPath",
CURRENT_USER_ID = "currentUserId",
FIRST_LOGIN = "firstLogin",
REQUIRED_MODAL_SHOWN_TIME = "requiredModalShownTime",
RECONFIRM_USER_CONTEXT_SHOWN_TIME = "reconfirmStageExamBoardShownTime",
LOGIN_OR_SIGN_UP_MODAL_SHOWN_TIME = "loginOrSignUpModalShownTime",
Expand Down
38 changes: 9 additions & 29 deletions src/app/state/actions/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
EventTypeFilter,
history,
isDefined,
isFirstLoginInPersistence,
KEY,
persistence,
QUESTION_ATTEMPT_THROTTLED_MESSAGE,
Expand Down Expand Up @@ -259,15 +258,7 @@ export const updateCurrentUser =
});
dispatch(requestCurrentUser() as any);

const isFirstLogin = isFirstLoginInPersistence() || false;
if (isFirstLogin) {
persistence.session.remove(KEY.FIRST_LOGIN);
if (redirect) {
history.push(persistence.pop(KEY.AFTER_AUTH_PATH) || "/", {
firstLogin: isFirstLogin,
});
}
} else if (!editingOtherUser) {
if (!editingOtherUser) {
dispatch(
showToast({
title: "Account settings updated",
Expand Down Expand Up @@ -312,13 +303,7 @@ export const registerUser =
dispatch({ type: ACTION_TYPE.USER_REGISTRATION_REQUEST });
const currentUser = await api.users.updateCurrent(newUser, userPreferences, null, userContexts, recaptchaToken);
dispatch({ type: ACTION_TYPE.USER_REGISTRATION_RESPONSE_SUCCESS, user: currentUser.data });
dispatch(requestCurrentUser() as any);

const isFirstLogin = isFirstLoginInPersistence() || false;
if (isFirstLogin) {
persistence.session.remove(KEY.FIRST_LOGIN);
history.push(persistence.pop(KEY.AFTER_AUTH_PATH) || "/", { firstLogin: isFirstLogin });
}
changePage("/register/success");
} catch (e: any) {
dispatch({ type: ACTION_TYPE.USER_REGISTRATION_RESPONSE_FAILURE, errorMessage: extractMessage(e) });
}
Expand Down Expand Up @@ -392,11 +377,12 @@ export const logInUser =
};

export const upgradeAccount =
(params: { verificationDetails: string; otherInformation?: string }) => async (dispatch: Dispatch<Action>) => {
(params: { verificationDetails: string; userEmail: string; otherInformation?: string }) =>
async (dispatch: Dispatch<Action>) => {
dispatch({ type: ACTION_TYPE.ACCOUNT_UPGRADE_SEND_REQUEST });
try {
const pendingAccountUpgrade = await api.users.upgradeAccount(params);
dispatch({ type: ACTION_TYPE.ACCOUNT_UPGRADE_SEND_RESPONSE_SUCCESS, user: pendingAccountUpgrade.data });
await api.users.upgradeAccount(params);
dispatch({ type: ACTION_TYPE.ACCOUNT_UPGRADE_SEND_RESPONSE_SUCCESS });
} catch (e) {
const errorMessage = extractMessage(e as Error);
dispatch({ type: ACTION_TYPE.ACCOUNT_UPGRADE_SEND_RESPONSE_FAILURE, errorMessage: errorMessage });
Expand Down Expand Up @@ -481,7 +467,7 @@ export const handleProviderCallback =
});
}
const nextPage = persistence.pop(KEY.AFTER_AUTH_PATH);
history.push(nextPage || "/");
history.push(nextPage ?? "/account");
} catch (error: any) {
history.push("/auth_error", { errorMessage: extractMessage(error) });
dispatch({ type: ACTION_TYPE.USER_LOG_IN_RESPONSE_FAILURE, errorMessage: "Login Failed" });
Expand Down Expand Up @@ -649,7 +635,7 @@ export const authenticateWithTokenAfterPrompt =
}
}
};
export const authenticateWithToken = (authToken: string) => async (dispatch: AppDispatch, getState: () => AppState) => {
export const authenticateWithToken = (authToken: string) => async (dispatch: AppDispatch) => {
try {
dispatch({ type: ACTION_TYPE.AUTHORISATIONS_TOKEN_APPLY_REQUEST });
await api.authorisations.useToken(authToken);
Expand All @@ -666,14 +652,8 @@ export const authenticateWithToken = (authToken: string) => async (dispatch: App
body: "You have granted access to your data.",
}) as any,
);
const state = getState();
// TODO currently this is not necessary because we are not on the correct tab after being told to log in
// user.firstLogin is set correctly using SSO, but not with Segue: check session storage too:
if ((state && state.user && state.user.loggedIn && state.user.firstLogin) || isFirstLoginInPersistence()) {
// If we've just signed up and used a group code immediately, change back to the main settings page:
history.push("/account");
}
dispatch(closeActiveModal() as any);
history.push("/account#profile");
} catch (e) {
dispatch({ type: ACTION_TYPE.AUTHORISATIONS_TOKEN_APPLY_RESPONSE_FAILURE });
dispatch(
Expand Down
7 changes: 2 additions & 5 deletions src/app/state/middleware/notificationManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,18 @@ export const notificationCheckerMiddleware: Middleware =
}

if (isDefined(user)) {
const firstLogin = persistence.session.load(KEY.FIRST_LOGIN);
// email confirmation modal to take precedence over other modals, only for teacherPending accounts
if (
needToVerifyEmail(user.teacherPending, user.emailVerificationStatus) &&
!withinLast2Minutes(persistence.load(KEY.EMAIL_CONFIRMATION_MODAL_SHOWN_TIME)) &&
firstLogin === null
!withinLast2Minutes(persistence.load(KEY.EMAIL_CONFIRMATION_MODAL_SHOWN_TIME))
) {
persistence.save(KEY.EMAIL_CONFIRMATION_MODAL_SHOWN_TIME, new Date().toString());
dispatch(openActiveModal(emailConfirmationModal));
}
// Required account info modal - takes precedence over stage/exam board re-confirmation modal
if (
isDefined(state.userPreferences) &&
!allRequiredInformationIsPresent(user, state.userPreferences, user.registeredContexts) &&
firstLogin === null
!allRequiredInformationIsPresent(user, state.userPreferences, user.registeredContexts)
) {
dispatch(openActiveModal(requiredAccountInformationModal));
}
Expand Down
1 change: 0 additions & 1 deletion src/app/state/middleware/userConsistencyChecker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ export const userConsistencyCheckerMiddleware: Middleware = (api: MiddlewareAPI)
clearCurrentUser();
break;
case ACTION_TYPE.USER_LOG_IN_RESPONSE_SUCCESS:
case ACTION_TYPE.ACCOUNT_UPGRADE_SEND_RESPONSE_SUCCESS:
case ACTION_TYPE.CURRENT_USER_RESPONSE_SUCCESS:
setCurrentUser(action.user, api);
break;
Expand Down
Loading

1 comment on commit 1639075

@github-actions
Copy link

Choose a reason for hiding this comment

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

Lines Statements Branches Functions
Coverage: 50%
50.23% (8457/16835) 32.92% (4018/12204) 39.47% (1416/3587)

Please sign in to comment.