Alvax10 / e-commerce_backend

dwf-m9-final.vercel.app
0 stars 0 forks source link

Feedback #1

Open EzequielVilla opened 2 years ago

EzequielVilla commented 2 years ago

Buenas Alva!

Estuve mirando el codigo sin probar los endpoints, pero te recomendaria que hagas unos cambios importantes igual.

-Primero el codigo no lo mandes en el res de createAuth, igual ahora esta bien porque te ahorraba el tiempo de ir a gmail y buscar el codigo, pero dejalo solo con el mail. -Estas llamando a models desde los endpoints, eso si esta mal, acordate de hacer eso siempre desde los controllers, que es donde tiene que ir a parar toda la logica. -Otra cosa es la funcion getLista, no entendi la finalidad, es un objeto inventado que tiene... apellidos, nombres, estaria bueno un comentario o algo porque como dije no entendi el uso que tendria. El endoint en si es raro, tenes una funcion, interna (getOffsetAndLimitFromReq), despues ek req.method == "get" y en los otros endpoints no haces nada de todo eso

Respecto a lo ultimo mencionado, la funcion getOffsetAndLimitFromReq la tenes en el controller ya definida

image

Eso tambien, se entiende pero seria mejor llamarlo de la forma que veniamos haciendo como postHandler, getHandler, etc y ahi especificar, para mejor interpretracion, porque el nombre getCartPost no entiendo cuando lo leo si hace un get o un post, lo mismo el delete. Tambien fijate que a tus constantes las llamaste productDeleted en ambos metodos (y uno es para el get)

-Algo que estoy viendo ahora y sin probar, creo que directamente no funciona tu auth. Si la logica no me falla fijate como procedes: 1- auth/index: Si alguien deja el email directamente le mandas el codigo 2- auth/token: Llamas a findByEmailAndCode, si no lo encuentra dice que o el email o el codigo es incorrecto, pero nunca llama a un "create lo que sea" Esto si es importante, no lo probe, pero a la vista no parece estar bien.

image

PD: Lo mismo que te dije antes, a esta funcion mejor llamala postHandler porque tenes una funcion en models/auth definida de la misma forma, que nunca en el codigo lo llamas.

Opcional: Las funciones cortas de algunos controllers podrias pasarlos a libs. Fijate si te parece pasar algunas funciones de controllers a libs, o juntar toda la logica de algo en un solo modulo, te lo dejo para que lo resuelvas vos jaja

+Bien los middlewares, verificando rapido los yup

-Te lo dejo en necesita ajustes, probalo por favor antes de reenviarlo.

Alvax10 commented 2 years ago

Buenas eze, gracias por el feedback, te voy respondiendo cada cosa que me pusiste por puntos:

1- Mi cuenta de sendgrid sigue en revisión, ya mandé el mail poniendo que cambié la contra y borré la API Key para que me devuelvan la cuenta, entonces por el momento (como no van a llegar los mails) dejé en el res el código, pero cuando me devuelvan la cuenta lo voy a sacar.

2- Ahora voy a corregir lo de los models, solamente en 3 archivos los llamo en los endpoint, asique los voy a pasar al controller.

3- El de lista es para mostrar el funcionamiento de offset y limit, aunque el endpoint de productos también la tiene, por eso la puse también en el controller a la función, si querés saco el archivo lista.

4- Si eze, me confundí nombradolo jaja sería getCart, pero dale, voy llamando a las funciones como hacíamos anteriormente. También voy a corregir ese nombre de variable duplicada.

5- El endpoint si anda, lo probé perfectamente en local y producción. El endpoint /auth/ crea o encuentra al usuario y le manda el código, luego /auth/token recibe el mail y cheuquea que coincida mail y token, (esto sin querer lo dejé en el endpoint pero lo voy a pasar al controller) entonces si el código y el mail coinciden, te manda la token, sino, te pone que falló.

6- De la forma que te lo mandé está completamente funcional, voy a corregir esos errores, probarlo en prod y local y lo vuelvo a mandar.

Gracias @EzequielVilla !

EzequielVilla commented 2 years ago

Alva! Si, fijate respecto al punto 5 (menos mal que los enumeraste) que hay algo ahi que me llama la atencion, y es que nunca llamas a createAuth, te dejo la cap

image

Si esa funcion nunca se llama entonces no se hace el collection.add(data)

Alvax10 commented 2 years ago

Sirvió de algo enumerarlas jaja, fijate te mando foto de la base de datos y en que momento de código creo el Auth.

En base de datos: Captura de pantalla de 2022-04-11 21-45-07 Captura de pantalla de 2022-04-11 21-45-13

En el código, la función auth/index crea al usuario de la siguiente forma:

1- Se invoca la función sendCode del controller: Captura de pantalla de 2022-04-11 21-46-02

2- Esa misma función invoca a la función findOrCreateUser (que también está en el controller) para chequear si el usuario existe o no y así no crearlo millones de veces en la base de datos, después de chequear, manda el código: Captura de pantalla de 2022-04-11 21-46-07

Esta es la función que invoca (se llama findOrCreateUser pero también crea al Auth al mismo tiempo que user, para no hacer funciones de más, además, ambas se van a crear siempre juntas): Captura de pantalla de 2022-04-11 21-48-16

Espero que se haya entendido :D ya arreglé todos los errores que me dijiste, solo me queda chequearlos a todos devuelta, saludoss

EzequielVilla commented 2 years ago

Peeeerfecto, no pude seguir el hilo de donde se creaba el auth pero ahi quedo clarito! Daaale , despues vuelvo a chequear el codigo y los endpoints!

Alvax10 commented 2 years ago

@EzequielVilla fijate que en la ultima foto, (función de findOrCreateAuth) si chequeé que auth no existe, se ve adentro del if el const newAuth = await Auth.createAuth() (En la línea 27) y toda esta función es invocada por sendCode, entonces en ese momento se crea el Auth :D