Skip to content

Comments

WIT-218: forgot password feature#99

Open
Merlte wants to merge 13 commits intomainfrom
new-feature/WIT-218/forgot-password
Open

WIT-218: forgot password feature#99
Merlte wants to merge 13 commits intomainfrom
new-feature/WIT-218/forgot-password

Conversation

@Merlte
Copy link

@Merlte Merlte commented Oct 21, 2024

No description provided.

@linear
Copy link

linear bot commented Oct 21, 2024

@railway-app
Copy link

railway-app bot commented Oct 21, 2024

This PR was not deployed automatically as @Merlte does not have access to the Railway project.

In order to get automatic PR deploys, please add @Merlte to your team on Railway.

doForgotPassword(reqData, setMessage, setError).finally(() => setLoading(false));
};

const initForgotPassword = async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

async is not utilised, can remove


useEffect(() => {
setLoading(true);
initForgotPassword();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This useEffect sets loading to true and to false immediately. Not sure why this was implemented.

email: e.currentTarget.userEmail.value,
}

doForgotPassword(reqData, setMessage, setError).finally(() => setLoading(false));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of passing in the setters from useState, could define a method that takes in an argument of what to set for the targeted variable.

Example

const updateMessage = (message: string) => {
  setMessage(message)
}

However, better way to further handle this is probably to use .then() to check for response and use setMessage() to set the message based on said response.

const data = await res.json();

if (res.ok) {
setMessage(data.message);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ideally shouldn't update the frontend states directly inside of methods that handles request to backend


// NOTE: re-using database because of tight deadlines
// token stores max size 32. Chose to use zid over email to fit length.
const token = btoa(Math.floor(Math.random() * 99999999999999 + 1) + zid.rows[0].zid);
Copy link
Collaborator

Choose a reason for hiding this comment

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

In generating the token, I believe we should stick maybe to the same unique id (uuidv4)?

return res.status(400).send({ message: "Invalid/expired token" });
}

const decoded = atob(token);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same for here for tokens as well

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