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

feature/edit-address #121

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
Open

feature/edit-address #121

wants to merge 11 commits into from

Conversation

PabloC5
Copy link

@PabloC5 PabloC5 commented Jun 22, 2023

Criado rota e formulário para editar o endereço e também criei um service para separar a lógica que vai salvar e comparar o endereço atual com o antigo para que a rota não fique poluída.

@PabloC5 PabloC5 requested a review from ronifabio June 22, 2023 03:47
Copy link
Contributor

@ronifabio ronifabio left a comment

Choose a reason for hiding this comment

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

  • Não funciona!
  • Assistir o vídeo de como cadastrar uma entidade
  • É preciso validar os dados antes de persistir, fazendo uso do DTO.

@@ -186,6 +186,59 @@ public ModelAndView showMyEmail(@PathVariable Long id) throws IOException {
return mv;
}

// teste rota endereço
/**
* Apresenta a tela de email do usuário.
Copy link
Contributor

Choose a reason for hiding this comment

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

Comentários não relacionados ao contexto

*/
@PostMapping("/salvar-endereco/{id}")
@RolesAllowed({RoleType.USER, RoleType.COMPANY})
public String saveAddress(
Copy link
Contributor

Choose a reason for hiding this comment

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

É preciso receber um DTO como parâmetro para persistência

try {
Optional<User> oUser = this.userService.findById(id);
User user = oUser.get();
Optional<City> cities = this.cityService.findById(user.getAddress().getCity().getId());
Copy link
Contributor

Choose a reason for hiding this comment

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

Usa-se plural quando é lista, neste caso, quando é Optional, usa-se: oCity

}
}

public void editAddress(Long id, HttpServletRequest request) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Não se faz uso do HttpServletRequest para pegar os parâmetros do formulário. O formulário é mapeado a um DTO. Ver os vídeos.

City cityUser = cities.get();
Address addressEdit = new Address();
addressEdit.setNeighborhood(request.getParameter("neighborhood"));
addressEdit.setStreet(request.getParameter("street"));
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Não se usa getParameter em Spring

…mudança do getParameter para capturar os valores do formulario usando o dto e mais alguns ajustes como remoção de comentarios etc.
@ronifabio ronifabio changed the title Feature/edit address feature/edit-address Jun 27, 2023

City cityMidDTO = oCity.get();

Address addressFullDTO = new Address();
Optional<User> oUser = this.userService.findById(id);
Copy link
Contributor

Choose a reason for hiding this comment

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

Faltou verificar se o usuário está logado.
E também se o id passado na URL corresponde ao id do usuário logado para ele não alterar dado de outro usuário

user.setAddress(addressEdit);
this.userService.save(user);
}
user.getAddress().setStreet(dto.getStreet().trim());
Copy link
Contributor

Choose a reason for hiding this comment

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

Usar o Mapper para copiar os dados do DTO para entidade.
Como é edição, o id do Address já existe

HttpServletRequest request,
RedirectAttributes redirectAttributes)
RedirectAttributes redirectAttributes,
@Validated(AddressDTO.RequestUserAddressInfoGroupValidation.class) AddressDTO dto,
Copy link
Contributor

Choose a reason for hiding this comment

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

Não precisa deste grupo: RequestUserAddressInfoGroupValidation

throws IOException {
this.addressService.editAddress(id, request);

Copy link
Contributor

Choose a reason for hiding this comment

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

Faltou usar o errors para mandar os erros de validação ao cliente, quando houver

throws IOException {
this.addressService.editAddress(id, request);

this.addressService.editAddress(id, dto, errors, httpSession);

redirectAttributes.addFlashAttribute("msg", "Endereço editado com sucesso");
Copy link
Contributor

Choose a reason for hiding this comment

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

Sempre vai mandar mensagem de sucesso, não está certo

throws IOException {
this.addressService.editAddress(id, request);

this.addressService.editAddress(id, dto, errors, httpSession);
Copy link
Contributor

Choose a reason for hiding this comment

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

Poderia deixar no próprio controller, principalmente as validações e a emissão de exception ou encaminhamento em caso de erro

…eixei comentado pois esta com um erro que deixei um comentario no codigo
Copy link
Contributor

@ronifabio ronifabio left a comment

Choose a reason for hiding this comment

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

  • Colocar todos os campos input do formulário com disabled.
  • Ao clicar no botão Editar, remover o modo disabled. Ou seja, o usuário poderá editar os valores. Também esconder o botão Editar e mostrar o botão SALVAR.
  • O editar é type button e o salvar é type submit, que será usado para submter o formulário.

@@ -209,13 +206,51 @@ public ModelAndView showMyAddress(@PathVariable Long id) throws IOException {

CityMidDTO testeNovo = userDTO.getAddress().getCity();
System.out.println(testeNovo);
CityDTO testeCity = new CityDTO();
// System.out.println(userDTO.getAddress().getCity());
Optional<City> cities = this.cityService.findById(testeNovo.getId());
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Quando é Optional, usa o prefixo o antes do nome da variável.
  • Ficaria oCity
  • Não se usa plural quando e apenas uma unidade, então, é oCity e não cities.

@@ -209,13 +206,51 @@ public ModelAndView showMyAddress(@PathVariable Long id) throws IOException {

CityMidDTO testeNovo = userDTO.getAddress().getCity();
System.out.println(testeNovo);
Copy link
Contributor

Choose a reason for hiding this comment

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

Não usar português

RedirectAttributes redirectAttributes)
throws IOException {
System.out.println("realmente um teste aqui");
Optional<User> oUser = this.userService.findById(id);
Copy link
Contributor

Choose a reason for hiding this comment

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

Verificar se o id recebido por parâmetro é o mesmo do usuário logado.

* @return
* @throws IOException
*/
@PostMapping("/salvar-endereco/{id}")
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Usar PATCH ao invés de POST
  • Está atualizando parte do usuário, sendo que o id do usuário permanece, só muda o endereço.

System.out.println("realmente um teste aqui");
Optional<User> oUser = this.userService.findById(id);
User user = oUser.get();
Optional<City> cities = this.cityService.findById(user.getAddress().getCity().getId());
Copy link
Contributor

Choose a reason for hiding this comment

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

É oCity

System.out.println("realmente um teste aqui");
Optional<User> oUser = this.userService.findById(id);
User user = oUser.get();
Optional<City> cities = this.cityService.findById(user.getAddress().getCity().getId());
Copy link
Contributor

Choose a reason for hiding this comment

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

Não pode pegar a mesma cidade que já está no usuário e colocar no novo endereço.
Vai que o usuário mudou de cidade.
Você precisa pegar a cidade que ele colocou no campo cidade no formulário.

@@ -61,13 +61,13 @@
</div>
<div class="row">
<div class="input-field col s8 offset-s2">
<input id="city" name="city" type="text" placeholder="Cidade" class="validate">
<input id="city" name="city" value="${city.name}" type="text" placeholder="Cidade" class="validate">
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Colocar as cidades em um campo select, não deixar em um input.
  • Apenas listar as cidades cadastradas no sistema, ou seja, fazer um findAll em City e lista no select.

<label for="city">Cidade</label>
</div>
</div>
<div class="row">
<div class="input-field col s8 offset-s2">
<input id="state" name="state" type="text" placeholder="Estado"
<input id="state" name="state" value="${city.state.name}" type="text" placeholder="Estado"
class="validate">
<label for="state">Estado</label>
Copy link
Contributor

Choose a reason for hiding this comment

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

Só listas os estados cadastrados no sistema.
Ver exemplo no cadastro de especialidades que usa um select de categorias
Está em ExpertiseController

PabloC5 and others added 2 commits July 2, 2023 21:27
… editar e o disabled nos campos, também arrumei algumas coisas refatorações no codigo do controller envolvendo verificações e também mantive o comentario e o mapper já que continuam sem funcionar mesmo usando o PatchMapping
@ronifabio ronifabio self-requested a review July 4, 2023 16:39
Copy link
Contributor

@ronifabio ronifabio left a comment

Choose a reason for hiding this comment

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

  • Faltou ajustar o CSS do formulário para deixar padronizado com a edição do telefone.
  • Ao buscar o endereço por CEP, verificar se a cidade e estado buscados existem no banco de dados. Fazer uma nova requisição ao nosso servidor. Caso a cidade e estado não sejam suportadas, mostrar uma mensagem de erro.
  • Nesta requisição, usar @responsebody para injetar o json diretamente na resposta.
  • Olhar o branch dynamic-statistics para exemplo de requisição ajax para o controller e obtenção da resposta.

@ronifabio
Copy link
Contributor

  • Não esquecer de atualizar o seu branch com as minhas modificações.
  • Com isso, basta fazer este tratamento com JS citado acima.

…, criei uma nova rotapost que retorna ResponseEntity
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