Skip to content

Conversation

@jerrrren
Copy link
Contributor

I created a public page that the backend will use to authenticate an account which the backend will send the the user's email. The specific account or code to be verified will be obtained using the URL parameters.
I also modified the home page with conditional rendering based on whether the account is verified.
Both home page and email verification page would be perform their actions on render using useEffect.

One thing I am unsure is if my hooks for the user information is done properly. I just copied the useUser hook from evoucher-frontend/src/api/user.ts and created a new verified user type

@jerrrren jerrrren requested a review from kevinmingtarja March 20, 2022 14:50
src/api/auth.ts Outdated

export const forgetPassword = (data: { email: string}) => {
export const forgetPassword = (data: {email: string}) => {
return request.post("/forgetpassword", data);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a matter of naming conventions. By convention (https://restfulapi.net/resource-naming/), REST API URLs should refer to nouns, rather than verbs. In this case, perhaps it would be better to name the URL something like /user/password/reset.

src/api/auth.ts Outdated
};

export const confirmEmail = (data:{uid:string}) => {
return request.post("/confirmEmail",data)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same goes for this one, perhaps /email/confirm would be more adhering to the convention. And also, it would be better to split multiple words into hyphens instead of using camelCase for naming REST URLs.

src/api/auth.ts Outdated


export const verificationEmail = (data:{uid:string}) => {
return request.post("/verifyEmail",data)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Comment on lines 1 to 37
import React, { ButtonHTMLAttributes } from 'react'

interface Props extends Omit<ButtonHTMLAttributes<HTMLButtonElement>, 'type' | 'htmlType'> {
id?: string
block?: boolean
children?: React.ReactNode
className?: string
disabled?: boolean
htmlType?: 'reset' | 'submit'
onClick?: () => void
size?: 'small' | 'large'
icon?: React.ReactNode
type?: 'danger' | 'success' | 'outlined' | 'text' | 'primary' | 'secondary'
vCenter?: boolean
isSubmit?: boolean
isLoading?: boolean
}

export const Button = ({
className,
size = 'large',
type = 'primary',
vCenter,
children,
disabled,
isSubmit,
isLoading = false,
...buttonProps
}: Props) => {
return (
<button {...buttonProps} disabled={Boolean(disabled)} type={isSubmit ? 'submit' : 'button'}>
{children}
</button>
)
}

export default Button
Copy link
Contributor

Choose a reason for hiding this comment

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

By the way, you can just use the button from chakra ui (https://chakra-ui.com/docs/components/form/button) instead of creating your own component from scratch, if you don't want to bother styling the components from scratch :D

src/api/auth.ts Outdated
}


export const verificationEmail = (data:{uid:string}) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to be more explicit about the function name. In this case, it might be a bit confusing as to what exactly this does. Is it sending the email itself, or is it doing other things related to verification email.

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