feat(UserService): Cambios en user endpoints and more #148
Conversation
- Se cambian los endpoints y se borra el middleware de user - Se arregla el error de regex en signup
| @@ -114,7 +111,6 @@ export default function Form() { | |||
| type='email' | |||
| id='email' | |||
| name='email' | |||
| pattern={mustBeMailAdress()} | |||
There was a problem hiding this comment.
porque se elimina el pattern aca? tenia entendido que queremos chequearlo pero con un pattern que no de error en consola que creo que es posible ( aunque no sea identico al del back )
There was a problem hiding this comment.
Porque no logre hace rque ande ningun otro, simpre tirarn error de syntaxis( que es lo que pasaba antes) y en realidad el mustBeMailAdress() no funcionaba tampoco y estaba corriendo solo el de type=email. Hice un poco de reaserch y el patter por defecto no esta tan mal y si no tenemos que controlar que sean mails se de la fing estamos bien con el de type
client/src/services/UserService.ts
Outdated
|
|
||
| export class UserService { | ||
| public static async getUser(id: string): Promise<User> { | ||
| return (await ApiCommunicator.getUser(id)) as User; | ||
| ////////////// AUTHENTICATION //////////////////////// |
There was a problem hiding this comment.
es necesario este comentario? con los nombres de la funciones creo que queda claro que es de autenticacion
There was a problem hiding this comment.
No, tenes razon, ya lo cambio a algo mas prolijo
| static api() { | ||
| return process.env.NEXT_PUBLIC_RAILS_API_URL; | ||
| } | ||
|
|
||
| static apiUrl() { | ||
| const RAILS_API_URL = process.env.RAILS_API_URL; | ||
|
|
There was a problem hiding this comment.
Como se diferencian estas dos variables de enotrno?
There was a problem hiding this comment.
en contenido? Nada, pero me daba terrible pereza poner el env en todos lados entonces lo puse en un metodo distinto ( todavia precismaos el otro metodo para no romper el resto)
client/src/services/UserService.ts
Outdated
|
|
||
| //////////////////////////////////////////////////////// | ||
|
|
||
| public static async getUser(session: Session): Promise<User> { |
There was a problem hiding this comment.
En otros PRs estamos siguiendo el patron de pasar los datos y el token en vez de toda la sesion. Deberiamos ser consistentes.
| | { | ||
| password: string; | ||
| } | ||
| ): Promise<Response> { |
There was a problem hiding this comment.
Creo que preferiria que esto me devuelva lo que es y no un objecto Response asi pelado. Siento que me estan leakeando las abstracciones y se me mezclan las capas.
There was a problem hiding this comment.
No entiendo, esta devolviendo lo que es, osea una response. En front preguntamos si esta bien y eso. Es horrible y esta linkeado con la idea de que se manejo como expetion todo y que no llegue como vos decis, pero esto queda para otro pr
| //Lo saco porque no es un regex valido y el type email ya funciona lo bastante bien en input | ||
| //no es perfecto pero no hay ningun regex que lo sea para mail |
There was a problem hiding this comment.
Creo que este comentario iba en el input y no aca. Hay un comentario al respecto.
client/src/services/UserService.ts
Outdated
| } | ||
|
|
||
| public static async modifyPassword( | ||
| id: string, | ||
| user: User, | ||
| currentPassword: string, | ||
| newPassword: string | ||
| ): Promise<any> { |
There was a problem hiding this comment.
Lo mismo con los any, hay chances de devolver cosas de dominio aca?
There was a problem hiding this comment.
Si, tenes razon en que ahora mismo deberia devolver Response
Mr-T-Smyth
requested review from
Cocofredo and
Frodo2412
and removed request for
Cocofredo and
marcegodoy
What && Why
Links