Add Firebase authentication for Merchant Sign Up #54
Conversation
There was a problem hiding this comment.
We should not couple firebase with our component! Here's what you can do:
- Initialise firebase in a
firebase.tsfile inweb/src. This file should contain thefirebase.initializeApp()line and things likeconst auth = firebase.auth;and export it and use that instead.
Other comments:
- Change to use react hooks and Functional Components! :D
- Using arrow functions will remove the need for binding so we can remove
autobindas well.
…y-group-buy into karen/firebase-sign-up
web/src/constants/sign-up-errors.ts
Outdated
| const PASSWORDS_DO_NOT_MATCH = 'Passwords do not match.'; | ||
|
|
||
| const INVALID_EMAIL = 'Please input a valid email address.'; | ||
| const WEAK_PASSWORD = 'Password should be at least 6 characters.'; |
There was a problem hiding this comment.
Why this error? Is there a error msg sent from firebase?
There was a problem hiding this comment.
Yeah firebase has these errors.
| setConfirmPassword(event.target.value); | ||
| }; | ||
|
|
||
| const handleVpaChange = (event: React.ChangeEvent<HTMLInputElement>) => { | ||
| event.target.value ? setVpaError('') : setVpaError(VPA_EMPTY); | ||
| event.target.value ? setVpaError('') : setVpaError(Errors.VPA_EMPTY); | ||
| setVpa(event.target.value); | ||
| }; | ||
|
|
||
| const handleSubmit = () => { |
There was a problem hiding this comment.
can you disable the submit button if there is some error
| setConfirmPasswordError(Errors.PASSWORDS_DO_NOT_MATCH); | ||
| if (!vpa) setVpaError(Errors.VPA_EMPTY); | ||
|
|
||
| if (!name || !email || !password || password !== confirmPassword || !vpa) { |
There was a problem hiding this comment.
Is it possible to avoid doing a second time check here? If there is a growing list of validation checks (e.g., 20 items), this is a long line of repetition.
There was a problem hiding this comment.
Changed to using library so we don't have to do this.
| try { | ||
| await firebaseAuth.createUserWithEmailAndPassword(email, password); | ||
| } catch (error) { | ||
| if (error.code === 'auth/invalid-email') { |
There was a problem hiding this comment.
Can change to switch statement?
"Generally speaking, if-else is best used when there are two discrete values or a few different ranges of values for which to test. When there are more than two discrete values for which to test, the switch statement is the most optimal choice" - source
There was a problem hiding this comment.
Okay! Yep, switch-case is better in this case.
…y-group-buy into karen/firebase-sign-up
…y-group-buy into karen/firebase-sign-up
We decided to use Firebase to handle authentication since it has a lot of authentication capabilities.
This PR adds Firebase code to handle authentication when merchant signs up.
This also stores merchant's account to Firebase so merchant can sign in with then same account in the future.
Some additional feature given by Firebase Authentication is checking email format, password strength and if existing user has the same email.