-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/login formular #3
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
base: develop
Are you sure you want to change the base?
Conversation
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.
Attention sur le nommage, 99% des commentaires c'est que ça, la logique est bonne mais c'est mal nommé
login, logout, register, ça peut être dans le même router "auth"
@@ -19,11 +19,12 @@ node_modules | |||
|
|||
# IDE - VSCode | |||
.vscode/* | |||
!.vscode/settings.json | |||
# !.vscode/settings.json |
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.
why?
req.isAuthenticated = true; | ||
req.userId = payload.userId; | ||
} catch (aterror) { | ||
console.error(aterror); |
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.
Surtout pas ! C'est une erreur qui va arriver très souvent, et c'est normal, si on console.log, ça va seulement spam les logs
A l'échelle de centaines d'utilisateurs, tu vas très vite surcharger les fichiers de logs, et donc le stockage de ton serveur
req.isAuthenticated = true; | ||
req.userId = payload.userId; | ||
} catch (rterror) { | ||
console.error('Refresh token is invalid', rterror); |
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.
idem
|
||
// If the refresh token is invalid, | ||
req.isAuthenticated = false; | ||
return; |
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.
Pourquoi return ? Si on arrive ici, le client qui fait la requête n'aura jamais de réponse
return db | ||
.selectFrom('user') | ||
.select(['user.id', 'user.password', 'user.email']) | ||
.where('user.email', '=', email || 'user.username ') |
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.
Pourquoi ?
return ( | ||
<div className='md:mx-16 md:my-24 md:h-full'> | ||
<div className='font-title my-10 text-3xl font-black md:text-6xl'> | ||
{' '} |
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.
espace vide
type ButtonInterface = { | ||
readonly title: string; | ||
}; |
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.
déjà déclarée dans un autre fichier de cette PR
const userLoginRouter = Router(); | ||
|
||
export const cookieRouterGet = Router(); |
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.
??
declare module 'Express' { | ||
// eslint-disable-next-line @typescript-eslint/consistent-type-definitions | ||
interface Request { | ||
userId?: number; | ||
} | ||
} |
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.
C'est déjà dans index.ts
import loginGuards from './middlewares/login.guards'; | ||
import loginMiddleware from './middlewares/login.middleware'; | ||
import userLoginRouter, { cookieRouterGet } from './subrouters/login-router'; | ||
import userLogout from './subrouters/logout-router'; |
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.
c'est n'importe quoi le nommage
No description provided.