Skip to content
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

Implementacion de CRUD para providers #136

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

Conversation

Werfran
Copy link

@Werfran Werfran commented Nov 18, 2021

Description

Se agregaron los métodos CRUD para proveedores

Issues

Opinion

No estoy seguro si el filtro para el seach deberia usar el metodo contains, pero lo use ya que es lo que se uso para el Store


public new Provider Create(Provider provider)
{
//if(NombreUnico(provider.Name) && EmailUnico(provider.Email) && TelefonoUnico(provider.Phone))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Al quedar comentado, no esta realizando estas validaciones. No es apropiado que no se realice algún tipo de validación al respecto.

}

[HttpPut("{id}")]
public void Put(string id, [FromBody] ProviderDTO value)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Que pasa si el parámetro id es distinto al del id contenido dentro del parámetro value?
Como se podría solucionar ese problema?

}

[HttpPost]
public ActionResult Post([FromBody] ProviderDTO value)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cual es la diferencia entre usar como respuesta un ActionResult, ActionResult<ProviderDTO> y ProviderDTO? Cual te parece más adecuado?
Te dejo links interesantes al respecto:
https://docs.microsoft.com/en-us/aspnet/core/web-api/action-return-types?view=aspnetcore-6.0
https://swagger.io/specification/

catch (Exception ex)
{
logger.LogCritical(ex.StackTrace);
return Ok(new { Success = false, Message = "The id is already in use" });
Copy link
Collaborator

Choose a reason for hiding this comment

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

Te parece correcto arrojar el mensaje que esta devolviendo en caso de excepción?
Que tipo de Http status code estas enviando en este caso? Te parece correcto?
Links:
https://restfulapi.net/http-status-codes/

…do para provider y store.

-Se arreglaron las observaciones hechas por los tutores.
-Ahora en el metodo put tiene que coincidir el id del provider del body con el de la URL.
-Se cambio a donde apunta el front para coincidir con la URL del back.
-Hay problema de cors, se agrego codigo pero se comento en el Startup.cs tratando de arreglarlo, no hubo caso.
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.

2 participants