Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

1ra Actividad - CRUD Provider Controller, ProviderDTO, ProviderService #131

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

1ra Actividad - CRUD Provider Controller, ProviderDTO, ProviderService #131

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ec801e6
CRUD Provider Controller, ProviderDTO, ProviderService
silvaelias1964 Nov 16, 2021
1a73142
Agregado de Search en ProviderController, ProviderSearchDTO, correcci…
silvaelias1964 Nov 17, 2021
e9c0779
algunas correcciones y mejoras en el ProviderController, ProviderSear…
silvaelias1964 Nov 18, 2021
5d5c192
Correcciones en el controller
silvaelias1964 Nov 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 54 additions & 29 deletions Stock.Api/Controllers/ProviderController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ public class ProviderController : ControllerBase

private readonly ProviderService service;
private readonly IMapper mapper;


public ProviderController(ProviderService service, IMapper mapper)
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fijate que el Startup configura un logger que podes inyectar y despues usar para registrar eventos y errores.

this.service = service ?? throw new ArgumentException(nameof(service));
this.mapper = mapper ?? throw new ArgumentException(nameof(mapper));
this.mapper = mapper ?? throw new ArgumentException(nameof(mapper));
}


Expand All @@ -35,14 +36,19 @@ public ProviderController(ProviderService service, IMapper mapper)
[HttpGet]
public ActionResult<IEnumerable<ProviderDTO>> Get()
{
logger.LogInformation("Mensaje Log de Get");
try
{
var result = service.GetAll();
return mapper.Map<IEnumerable<ProviderDTO>>(result).ToList();
var provider = service.GetAll();
if (provider == null)
{
return Ok(new { statusCode = "404", result = "No hay datos en Proveedores" });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

De acuerdo, ya lo cambié

}
return mapper.Map<IEnumerable<ProviderDTO>>(provider).ToList();
}
catch (Exception)
{
return StatusCode(500);
catch (Exception ex)
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totalmente de acuerdo, no lo habia pensado.

return BadRequest(new { statusCode = "400", errorMessage = ex.Message });
}
}

Expand All @@ -53,8 +59,21 @@ public ActionResult<IEnumerable<ProviderDTO>> Get()
/// <returns>A <see cref="ProviderDTO"/></returns>
[HttpGet("{id}")]
public ActionResult<ProviderDTO> Get(string id)
{
return Ok(mapper.Map<ProviderDTO>(service.Get(id)));
{
try
{
var provider = mapper.Map<ProviderDTO>(service.Get(id));
if (provider == null)
{
return Ok(new { statusCode = "404", result = "El Proveedor no fué encontrado" });
}
return Ok(provider);
}
catch (Exception ex)
{
return BadRequest(new { statusCode = "400", errorMessage = ex.Message });
}

}

/// <summary>
Expand All @@ -75,18 +94,23 @@ public Provider Post([FromBody] ProviderDTO value)
/// <param name="id">Provider id to edit.</param>
/// <param name="value">Prodvider information.</param>
[HttpPut("{id}")]
public void Put(string id, [FromBody] ProviderDTO value)
{
var provider = service.Get(id);
public ActionResult Put(string id, [FromBody] ProviderDTO value)
{
TryValidateModel(value);
try
{
var provider = service.Get(id);
if (provider == null)
{
return Ok(new { statusCode = "404", result = "El Proveedor no fué encontrado" });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aca lo mismo que el la linea 45. Con un return NotFound(); es suficiente

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Listo

}
mapper.Map<ProviderDTO, Provider>(value, provider);
service.Update(provider);
return Ok(new { statusCode = "200", result = "Proveedor modificado exitosamente" });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
catch (Exception ex)
{
var msg = ex.Message;
{
return BadRequest(new { statusCode = "400", result = ex.Message });
}
}

Expand All @@ -97,17 +121,23 @@ public void Put(string id, [FromBody] ProviderDTO value)
[HttpDelete("{id}")]
public ActionResult Delete(string id)
{
var provider = service.Get(id);

if (provider is null)
return NotFound();
try
{
var provider = service.Get(id);
if (provider is null)
return Ok(new { statusCode = "404", result = "El Proveedor no fué encontrado" });

service.Delete(provider);
return Ok();
service.Delete(provider);
return Ok(new { statusCode = "200", result = "Proveedor eliminado exitosamente" });
}
catch (Exception ex)
{
return BadRequest(new { statusCode = "400", result = ex.Message });
}
}

/// <summary>
/// Search by Id, Name, Phone, Email
/// Search by Name, Phone, Email
/// </summary>
/// <param name="model">Provider information.</param>
/// <returns>A <see cref="ProviderSearchDTO"/></returns>
Expand All @@ -117,14 +147,6 @@ public ActionResult Search([FromQuery] ProviderSearchDTO model)
{
Expression<Func<Provider, bool>> filter = x => !string.IsNullOrWhiteSpace(x.Id);

// Search by Id
if (!string.IsNullOrWhiteSpace(model.Id))
{
filter = filter.AndOrCustom(
x => x.Id.ToUpper().Contains(model.Id.ToUpper()),
model.Condition.Equals(ActionDto.AND));
}

// Search by Name
if (!string.IsNullOrWhiteSpace(model.Name))
{
Expand All @@ -149,8 +171,11 @@ public ActionResult Search([FromQuery] ProviderSearchDTO model)
model.Condition.Equals(ActionDto.AND));
}


var provider = service.Search(filter);

if(provider.Count()==0)
return Ok(new { statusCode = "404", result = "No se encontró información del Proveedor" });

return Ok(provider);
}
}
Expand Down
6 changes: 2 additions & 4 deletions Stock.Api/DTOs/ProviderSearchDTO.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,14 @@ namespace Stock.Api.DTOs
{
public class ProviderSearchDTO
{

public string Id { get; set; }


public string Name { get; set; }

public string Phone { get; set; }

public string Email { get; set; }

public List<Product> OfferedProducts { get; set; }
//public List<Product> OfferedProducts { get; set; }

public ActionDto Condition { get; set; }
}
Expand Down
10 changes: 9 additions & 1 deletion Stock.Api/MapperProfiles/ModelProfile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ public ModelProfile()
// .ForMember(s => s.Id, opt => opt.Ignore())
// .ForMember(s => s.ProductType, opt => opt.Ignore());



CreateMap<Provider, ProviderDTO>()
.ForMember(c => c.Id, opt => opt.MapFrom(s => s.Id))
.ForMember(c => c.Name, opt => opt.MapFrom(s => s.Name))
Expand All @@ -37,7 +39,13 @@ public ModelProfile()
//.ReverseMap()
//.ForPath(s => s.OfferedProducts, opt => opt.Ignore());

}
CreateMap<Provider, ProviderSearchDTO>()
.ForMember(c => c.Name, opt => opt.MapFrom(s => s.Name))
.ForMember(c => c.Phone, opt => opt.MapFrom(s => s.Phone))
.ForMember(c => c.Email, opt => opt.MapFrom(s => s.Email));


}
}


Expand Down