-
Notifications
You must be signed in to change notification settings - Fork 17
feat: implement control plane #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: main
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.
La verdad que me ha gustado mucho ! solo unos comentarios tontos para mejorarlo un poco más pero más hallá de ello... GENIAL !!
Y en cuanto a la config diferente, sí, tienes que tener cuidado y directamente desactivar el formateo por prettier y utilizar el de eslint del project porque el problema es que genera muchísimo ruido a la hora de revisar prs.
PD: No mergear, es solo a nivel de aprendizaje
password: 'password', | ||
} | ||
|
||
authenticatorProxyAdapter.getAuthDetails( |
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.
Esto está bien para probar de manera local, pero lo eliminaría para prod ya que está probado mediante los test que esto funcione correctamente con datos de prueba
it.concurrent('should get user permissions', async () => { | ||
//GIVEN | ||
|
||
const expectedResult: Permissions = { |
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.
Realmente no me acuerdo si lo había dicho en el vídeo sobre tener dos properties para los roles, pero es posible que con solo saber si es admin o no sea suficiente, ya que si no es admin... es user.
import jwt from 'jsonwebtoken' | ||
|
||
export class ControlPlane implements ForManagingAuthDetails { | ||
private secretKey: string = 'mySecretKey' |
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.
Dejar espacios entre unidades lógicas
private secretKey: string = 'mySecretKey' | |
private secretKey: string = 'mySecretKey' | |
|
||
return refreshToken | ||
} | ||
const token = generateToken({ email, password }, this.secretKey, '30m') |
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.
Dejar espacios entre unidades lógicas
const token = generateToken({ email, password }, this.secretKey, '30m') | |
const token = generateToken({ email, password }, this.secretKey, '30m') | |
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.
'30m' podría ser una variable que puede ser modificable por algún método para que sea congruente en toda la app
Hola, aun no entiendo muy bien que signfica tuti.
Conocí tu comunidad y el canal hace una semana y tenia conocimiento nulo en clean architecture y mucho menos en la hexagonal, pero me esta llamando mucho la atención. Se que varias cositas estaré haciendo mal, pero lo hice para empezar a cogerle el ritmo a estas nuevas arquitecturas, para mi. Disculpa que cogiera el PR para esto pero te agradezco mucho todo lo que enseñas a diario. un saludo!
ps. volvi a subir el pr porque estaba jugando con la implementacion de trpc y ahora lo dejo actualizado con esos cambios.
sorry que mi prettier config es distinta a la que usas, quedo pendiente para cambiarlo, veo que se hicieron unos cambios que no son para el pr.