Skip to content

API // Revisión de 2da entrega: sesiones#4

Open
nmicht wants to merge 1 commit intoColeMacGrath:masterfrom
nmicht:rev2.0
Open

API // Revisión de 2da entrega: sesiones#4
nmicht wants to merge 1 commit intoColeMacGrath:masterfrom
nmicht:rev2.0

Conversation

@nmicht
Copy link
Copy Markdown

@nmicht nmicht commented Nov 29, 2018

En general todo se ve bien, solo hay detalles de mejora:

  • Evitar cargar auth en los controllers
  • Documentación de clases y métodos
  • TIP: En el readme, el link a la colección de postman puede ser un link directo de postman, de este modo se abre directo en postman ;)
- Esta clase debe estar documentada, así como cada uno de sus métodos. __FIXME__ [middlewares/auth.js](middlewares/auth.js)
- esta clase y sus metodos deberian estar documentados __FIXME__ [mail/index.js](mail/index.js)
- Los ids no deberian ser arreglos, es un poco extraño ver que tienen que acceder al que esta en la posicion 0 __FIXME__ [middlewares/auth.js](middlewares/auth.js)
- Los ids no deberian ser arreglos, es un poco extraño ver que tienen que acceder al que esta en la posicion 0 __FIXME__ [middlewares/auth.js](middlewares/auth.js)
- Para evitar acceder a game[0] se debe tener un metodo para obtener el primero __FIXME__ [middlewares/auth.js](middlewares/auth.js)
- para evitar estar metiendo metodos de auth en los controllers, __FIXME__ [middlewares/auth.js](middlewares/auth.js)
- se pueden cargar los dos middlewares en una linea __FIXME__ [routes/users.js](routes/users.js)
- si el metodo lo que valida es que sea admin, deberia llamar isAdmin en lugar de havePermissions __FIXME__ [middlewares/auth.js](middlewares/auth.js)
- Todos los modelos se pueden cargar en una sola linea __FIXME__ [middlewares/auth.js](middlewares/auth.js)

@nmicht nmicht changed the title Revisión 2da entrega: sesiones API // Revisión de 2da entrega: sesiones Nov 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant