-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add creation user required step #102
Changes from all commits
644d861
4255c24
c9d1ada
f94c419
511a0e9
0e2e53a
976f648
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
'use client'; | ||
import { Grid } from '@mui/material'; | ||
import { usePathname } from 'next/navigation'; | ||
import React, { useEffect } from 'react'; | ||
|
||
import { AddUserDialog } from './dialogs/AddUserDialog'; | ||
import { useAuth } from '../context/AuthProvider'; | ||
import { apiUser } from '../services/endpoints/user.class'; | ||
|
||
export const RequiredSteps = ({ children }: { children: React.ReactNode }) => { | ||
const { user, isAuthenticated } = useAuth(); | ||
const pathname = usePathname(); | ||
|
||
const [isCreateUserDialogOpened, setIsCreateUserDialogOpened] = | ||
React.useState(false); | ||
const [hasUser, setHasUser] = React.useState(false); | ||
const isAnonymous = !isAuthenticated && pathname === '/viewer'; | ||
|
||
useEffect(() => { | ||
const fetchUser = async (id: string) => | ||
apiUser | ||
.getUser(id) | ||
.then(({ data }) => { | ||
setHasUser(true); | ||
return data; | ||
}) | ||
.catch(() => { | ||
setHasUser(false); | ||
setIsCreateUserDialogOpened(true); | ||
}); | ||
|
||
if (user?.id) fetchUser(user.id); | ||
}, []); | ||
|
||
return ( | ||
<Grid minHeight={'calc(100vh - 137px)'}> | ||
{isAnonymous || hasUser ? ( | ||
children | ||
) : ( | ||
<AddUserDialog | ||
open={isCreateUserDialogOpened} | ||
onClose={() => { | ||
setIsCreateUserDialogOpened(false); | ||
}} | ||
callback={() => { | ||
setHasUser(true); | ||
}} | ||
/> | ||
Comment on lines
+40
to
+48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve AddUserDialog prop naming and callback handling. The current implementation of AddUserDialog props could be improved for clarity and maintainability. Consider the following changes:
Here's a suggested refactor: <AddUserDialog
open={isCreateUserDialogOpened}
- onClose={() => {
- setIsCreateUserDialogOpened(false);
- }}
- callback={() => {
+ onClose={() => setIsCreateUserDialogOpened(false)}
+ onUserCreated={() => {
setHasUser(true);
+ setIsCreateUserDialogOpened(false);
}}
/> This change improves the readability and maintainability of the component.
|
||
)} | ||
</Grid> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
'use client'; | ||
|
||
import { Send } from '@mui/icons-material'; | ||
import { Dialog, DialogContent, Grid, Alert, TextField } from '@mui/material'; | ||
import { FC, useEffect, useState } from 'react'; | ||
import { useForm } from 'react-hook-form'; | ||
|
||
import { useAuth } from '@/app/context/AuthProvider'; | ||
import { apiUser } from '@/app/services/endpoints/user.class'; | ||
import { CreateUserDto } from '@/domain/dtos/user'; | ||
|
||
import { StyledButton } from '../StyledButton'; | ||
import { StyledDialogActions } from '../StyledDialogActions'; | ||
import { StyledDialogContent } from '../StyledDialogContent'; | ||
import { StyledDialogTitle } from '../StyledDialogTitle'; | ||
|
||
interface AddUserDialogProps { | ||
open?: boolean; | ||
onClose?: () => void; | ||
callback?: () => void; | ||
} | ||
Comment on lines
+17
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider refining the While the current interface allows for flexibility, consider the following improvements for better type safety:
Example refactored interface: interface AddUserDialogProps {
open: boolean;
onClose: () => void;
callback?: () => void;
} This change would enforce better prop usage and potentially prevent runtime errors. |
||
|
||
export const AddUserDialog: FC<AddUserDialogProps> = ({ | ||
open, | ||
onClose, | ||
callback, | ||
}) => { | ||
const { session } = useAuth(); | ||
const [hasError, setHasError] = useState(false); | ||
const [disableButton, setDisableButton] = useState(false); | ||
|
||
const { | ||
reset, | ||
register, | ||
formState: { errors }, | ||
resetField, | ||
handleSubmit, | ||
} = useForm<CreateUserDto>(); | ||
Comment on lines
+32
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance type safety for the To improve type safety and autocompletion, consider using a more specific type for the Example: interface AddUserFormData {
userId: string;
patientId: string;
}
const {
reset,
register,
formState: { errors },
resetField,
handleSubmit,
} = useForm<AddUserFormData>(); This change will ensure that the form fields and their types are explicitly defined, improving type checking and developer experience.
Comment on lines
+23
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance type safety for the To improve type safety and autocompletion, consider using a more specific type for the Example: interface AddUserFormData {
userId: string;
patientId: string;
}
const {
reset,
register,
formState: { errors },
resetField,
handleSubmit,
} = useForm<AddUserFormData>(); This change will ensure that the form fields and their types are explicitly defined, improving type checking and developer experience. |
||
|
||
const onSubmitForm = async (data: CreateUserDto) => { | ||
setDisableButton(true); | ||
try { | ||
await apiUser | ||
.createUser(data) | ||
.then(() => { | ||
onClose?.(); | ||
callback?.(); | ||
}) | ||
.catch(() => { | ||
setHasError(true); | ||
setDisableButton(false); | ||
}); | ||
} catch (error) { | ||
console.error('Failed to create user:', error); | ||
} | ||
}; | ||
Comment on lines
+40
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance error handling and form state management in Consider the following improvements:
These changes will improve error handling, provide better feedback to users, and ensure the form is in a clean state after successful submission. |
||
|
||
useEffect(() => { | ||
if (open) { | ||
reset(); | ||
setHasError(false); | ||
setDisableButton(false); | ||
} | ||
}, [open]); | ||
|
||
useEffect(() => { | ||
if (session?.token?.sub) | ||
resetField('userId', { defaultValue: session.token.sub }); | ||
}, [session?.token?.sub]); | ||
Comment on lines
+66
to
+69
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add a safety check for the session token in the useEffect hook. To prevent potential runtime errors, consider adding a safety check: useEffect(() => {
if (session?.token?.sub) {
resetField('userId', { defaultValue: session.token.sub });
} else {
console.warn('Session token is undefined. Unable to set userId.');
// Optionally, set a default value or handle this case as needed
}
}, [session?.token?.sub, resetField]); This change will make the code more robust and prevent potential issues if the session token is unexpectedly undefined. |
||
|
||
return ( | ||
<Dialog open={!!open} fullWidth maxWidth="xs"> | ||
<form onSubmit={handleSubmit(onSubmitForm)}> | ||
<StyledDialogTitle>Verify user details</StyledDialogTitle> | ||
<DialogContent style={{ padding: '5px 8px' }}> | ||
<StyledDialogContent> | ||
{hasError && ( | ||
<Grid pb={2}> | ||
<Alert severity="error">The creation of a user is failed</Alert> | ||
</Grid> | ||
)} | ||
<TextField | ||
label="User id" | ||
error={!!errors.userId} | ||
disabled | ||
required | ||
inputProps={{ readOnly: true }} | ||
{...register('userId', {})} | ||
/> | ||
<TextField | ||
type="number" | ||
label="National identity number *" | ||
{...register('patientId', { | ||
setValueAs: (value) => value || undefined, | ||
})} | ||
/> | ||
Comment on lines
+90
to
+96
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add validation rules for the patientId field. The <TextField
type="number"
label="National identity number *"
error={!!errors.patientId}
helperText={errors.patientId?.message}
{...register('patientId', {
required: 'National identity number is required',
validate: (value) => {
// Add custom validation logic here
// For example, check if it's a valid length for a national ID
return value.length === 10 || 'Invalid national identity number';
},
setValueAs: (value) => value || undefined,
})}
/> This change will ensure that the |
||
</StyledDialogContent> | ||
</DialogContent> | ||
<StyledDialogActions sx={{ justifyContent: 'end' }}> | ||
<StyledButton | ||
type="submit" | ||
color="success" | ||
variant="contained" | ||
disabled={disableButton} | ||
> | ||
<Send sx={{ paddingRight: '4px', marginRight: '4px' }} /> | ||
Create | ||
</StyledButton> | ||
</StyledDialogActions> | ||
</form> | ||
</Dialog> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import { type AxiosInstance } from 'axios'; | ||
|
||
import { BaseApi, instance } from '../api.class'; | ||
import { | ||
EPath, | ||
type IPathMapTypes, | ||
type TBaseApiProps, | ||
type TPathToOperations, | ||
} from '../api.types'; | ||
|
||
export class User< | ||
TPath extends keyof IPathMapTypes, | ||
TOperations extends TBaseApiProps<TOperations> = TPathToOperations<TPath>, | ||
> extends BaseApi<TOperations> { | ||
constructor(protected readonly instance: AxiosInstance) { | ||
super(instance); | ||
} | ||
|
||
async createUser(data: TOperations['create']['req']) { | ||
return await this.create({ | ||
url: `/${EPath.users}`, | ||
data, | ||
}); | ||
} | ||
|
||
async getUser(userId: string) { | ||
return await this.get({ | ||
url: `/${EPath.users}/${userId}`, | ||
}); | ||
} | ||
} | ||
|
||
export const createApiUser = () => new User<EPath.users>(instance); | ||
export const apiUser = createApiUser(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @yassinedorbozgithub, I suggest we import everything we need from react once and for all, to keep the code aligned with other files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure we will do that