Skip to content

fix. errors of register login#47

Open
FBagdeli wants to merge 9 commits into
mainfrom
farshad#25-errors_of_register_login
Open

fix. errors of register login#47
FBagdeli wants to merge 9 commits into
mainfrom
farshad#25-errors_of_register_login

Conversation

@FBagdeli
Copy link
Copy Markdown
Collaborator

No description provided.

@aws-amplify-eu-west-2
Copy link
Copy Markdown

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-47.d1ytr8ocn5wsgw.amplifyapp.com

Comment thread src/context/auth.js Outdated

if(password.length < 1) return 'Password is required'
if(password.length < minLength ) return 'Password should be at least 8 characters long'
if(password.length > 24) return 'Password should be less than 24 characters'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Probably wait for the database changes, if this is required surely could be used, but for right now, i guess this is not supposed to be added.

Comment thread src/context/auth.js Outdated
const handleRegister = async (email, password, setErrors) => {
try {
const validatedEmail = validationEmail(email)
if(validatedEmail !== true) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if(!validatedEmail)

Comment thread src/context/auth.js Outdated
if(!hasUppercase) return 'Password should have at least one uppercase letter'
if(!hasNumber) return 'Password should have at least one number'
if(!hasSpecialCharacter) return 'Password should have at least one special character'
return 'ValidPassword'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would return true or false for this function and just display 1 error message

Comment thread src/context/auth.js Outdated
throw new Error(validatedEmail)
}
const validatedPassword = validationPassword(password)
if(validatedPassword !== 'ValidPassword') {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

!validatedPassword

Comment thread src/context/auth.js Outdated
if(!hasUppercase) return 'Password should have at least one uppercase letter'
if(!hasNumber) return 'Password should have at least one number'
if(!hasSpecialCharacter) return 'Password should have at least one special character'
return 'ValidPassword'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

return true

@FBagdeli FBagdeli requested a review from Hamada-AB July 19, 2024 12:10
MyrtheDullaart
MyrtheDullaart previously approved these changes Jul 19, 2024
@FBagdeli FBagdeli removed the request for review from Hamada-AB July 19, 2024 12:16
Comment thread src/context/auth.js Outdated

const handleLogin = async (email, password, setErrors) => {

const passwordRequirments = 'Password must be at least 8 characters long and include at least one uppercase letter, one number, and one special character.'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Let's put these errors in a utils/errors.js file so they're not scattered around the app

Comment thread src/context/auth.js Outdated
const emailErrorMessage = "Please enter a valid email"

try {
const validatedEmail = validationEmail(email)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the formatting here looks wrong

Comment thread src/context/auth.js Outdated
navigate("/verification")
}
const handleRegister = async (email, password, setErrors) => {
const passwordRequirments = 'Password must be at least 8 characters long and include at least one uppercase letter, one number, and one special character.'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

let's put these in utils/errors.js

Comment thread src/context/auth.js Outdated
const emailErrorMessage = "Please enter a valid email"

try {
const validatedEmail = validationEmail(email)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

formatting doesn't look correct

Comment thread src/context/auth.js Outdated
localStorage.setItem('token', token)
navigate('/')
}
// localStorage.setItem('token', token)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

let's remove commented code

Comment thread src/pages/login/index.js Outdated
const onChange = (e) => {
const { name, value } = e.target;
setFormData({ ...formData, [name]: value });
setErrors('')
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

let's use null as a default value

Comment thread src/pages/login/index.js Outdated
@@ -38,10 +40,11 @@ const Login = () => {
label={"Password *"}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

check the formatting here - it doesn't look right

Comment thread src/pages/register/index.js Outdated
const { onRegister } = useAuth();
const [formData, setFormData] = useState({ email: "", password: "" });

const [errors, setErrors] = useState('')
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

let's use null as a default value

Comment thread src/pages/register/index.js Outdated
const onChange = (e) => {
const { name, value } = e.target;
setFormData({ ...formData, [name]: value });
setErrors('')
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

let's use null as a default value

Comment thread src/service/apiClient.js Outdated
if(res.status === 'fail') {
throw new Error(res.data.error)
}
return await login(email, password)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

no need for this await

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.

4 participants