1ra Actividad - CRUD Provider Controller, ProviderDTO, ProviderService #131
Conversation
…ón ModelProfile, providerservice
silvaelias1964
changed the title
| private readonly IMapper mapper; | ||
|
|
||
| public ProviderController(ProviderService service, IMapper mapper) | ||
| { |
There was a problem hiding this comment.
Fijate que el Startup configura un logger que podes inyectar y despues usar para registrar eventos y errores.
| var result = service.GetAll(); | ||
| return mapper.Map<IEnumerable<ProviderDTO>>(result).ToList(); | ||
| } | ||
| catch (Exception) |
There was a problem hiding this comment.
Si no vas a asignar la exception a ninguna variable y no vas a diferenciar entre tipos de Exception, no hace falta que aclares el tipo, es un "catch all" pero por lo general es mejor asignarle una variable y podes loggear algo de informacion adicional del error.
Tene en cuenta que el codigo de ejemplo en las entidades que ya estan creadas como Store no es perfecto y tiene lugar para que lo mejores!
There was a problem hiding this comment.
Listo coloque mejores mensajes de repuesta y capturé los exception message
| /// <returns>A <see cref="ProviderDTO"/></returns> | ||
| [HttpGet("{id}")] | ||
| public ActionResult<ProviderDTO> Get(string id) | ||
| { |
There was a problem hiding this comment.
Aqui tambien hay lugar para mejorar lo que ya existe, este metodo hace la busqueda por un id especifico, si ese ID no existe, podes devolver un 404 Not Found
| Expression<Func<Provider, bool>> filter = x => !string.IsNullOrWhiteSpace(x.Id); | ||
|
|
||
| // Search by Id | ||
| if (!string.IsNullOrWhiteSpace(model.Id)) |
There was a problem hiding this comment.
Fijate que el ID es unico, por lo cual no tendria sentido hacer un AND o un OR con otro property del proveedor; para hacer una busqueda por ID ya tenes el GET
There was a problem hiding this comment.
Dejé solo tres campos del provider, quité el Id.
Stock.Api/DTOs/ProviderSearchDTO.cs
Outdated
|
|
||
| public string Email { get; set; } | ||
|
|
||
| public List<Product> OfferedProducts { get; set; } |
There was a problem hiding this comment.
No seria mala idea tener la opcion de hacer una busqueda de proveedores que ofrezcan ciertos productos, pero actualmente el metodo search no esta usando esto asi que la property de este objeto no es necesaria. Implmentar esa busqueda seria un poco mas complejo y no hace falta que la hagas como parte del ejercicio inicial, pero si te animas podes intentarlo!
| var provider = service.GetAll(); | ||
| if (provider == null) | ||
| { | ||
| return Ok(new { statusCode = "404", result = "No hay datos en Proveedores" }); |
There was a problem hiding this comment.
Va mejor lo de devolver el not found, pero la idea es que sea el HttpStatus correcto, si haces un return Ok(); el response va a ser 200 OK on el json que dice "404".
En lugar de eso, con hacer simplemente return NotFound(); ya estas haciendo que el response sea 404 Not Found y no es necesario aclarar nada mas en el body del response
There was a problem hiding this comment.
De acuerdo, ya lo cambié
| { | ||
| return StatusCode(500); | ||
| catch (Exception ex) | ||
| { |
There was a problem hiding this comment.
Dependiendo del tipo de exception que captures, la respuesta va a ser un 400 Bad Request o un 500 Internal server error. La diferencia es que un 400 el error esta del lado del cliente, un 500 es un error del lado del servidor. Simplemente dejando la exception sin handlear haces que el cliente reciba el 500.
Por otra parte, el mensaje de error podes logearlo, pero sobre todo en entornos de produccion, no deberias devolverlo al cliente en el response porque podrias estar revelando alguna vulnerabilidad de seguridad.
There was a problem hiding this comment.
Totalmente de acuerdo, no lo habia pensado.
| var provider = service.Get(id); | ||
| if (provider == null) | ||
| { | ||
| return Ok(new { statusCode = "404", result = "El Proveedor no fué encontrado" }); |
There was a problem hiding this comment.
Aca lo mismo que el la linea 45. Con un return NotFound(); es suficiente
| mapper.Map<ProviderDTO, Provider>(value, provider); | ||
| service.Update(provider); | ||
| return Ok(new { statusCode = "200", result = "Proveedor modificado exitosamente" }); |
There was a problem hiding this comment.
Generalmente para los POST (creando un objeto nuevo) la respuesta es 201 Created, para los PUT (updates) y DELETE, se puede usar un 200 OK, o tambien se suele usar un 204 No Content.
Description
Creado el CRUD ProviderController, el modelo ProviderDTO y el servicio ProviderService