-
Notifications
You must be signed in to change notification settings - Fork 10
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
Feat Prioridad del Proyecto #14
base: main
Are you sure you want to change the base?
Conversation
Creacion del modelo Creacion del repository Creacion del service Creacion del DTO Creacion del Mapper Creacion del GlobalExceptionHandler Creacion de las distintas exceptions
|
||
@RestController | ||
@RequestMapping("/prioridad-proyecto") | ||
@CrossOrigin (origins = "*") |
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 es un riesgo de seguridad, permite todos los dominios de origen y solo estaria medianamente a salvo en una red privada. Dejo como acotacion.
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.
Si, la cuestion es que no sabia si manejaban las cors de forma global ya que no habia mas info sobre el proyecto completo
@Autowired | ||
private ResponseService responseService; | ||
|
||
@GetMapping({"/"}) |
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.
No es necesario agregar la barra si el mapping va al root, nuevamente solo como acotacion.
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.
Relativamente bien, solo convenciones en los controladores. El resto de la implementacion esta genial.
import java.util.List; | ||
|
||
@RestController | ||
@RequestMapping("/prioridad-proyecto") |
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.
El nombre de la ruta se deberia ajustar a algo mas acorde como /prioridades
return responseService.successResponse(prioridades, "OK"); | ||
} | ||
|
||
@GetMapping("/id/{id}") |
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 podria ser solamente /{id}
para respetar la convencion de nombre.
} | ||
|
||
|
||
@PostMapping({"/"}) |
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.
Mismo caso que el GET /
de arriba.
} | ||
|
||
|
||
@DeleteMapping("/id/{id}") |
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.
Mismo caso que el /{id}
arriba.
Implementamos: