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

Recuperação de Senha #374

Merged
merged 7 commits into from
May 25, 2022
Merged

Recuperação de Senha #374

merged 7 commits into from
May 25, 2022

Conversation

filipedeschamps
Copy link
Owner

Dando continuidade ao PR #358 de @gabrielew onde iniciei fazendo o squash dos commits e agora vou levar a feature até ao final e seguindo as sugestões que apareceram na issue de controle #324

@vercel
Copy link

vercel bot commented May 23, 2022

@gabrielew is attempting to deploy a commit to the TabNews Team on Vercel.

To accomplish this, @gabrielew needs to request access to the Team.

Afterwards, an owner of the Team is required to accept their membership request.

If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account.

models/recover.js Outdated Show resolved Hide resolved
pages/cadastro/recuperar/[token].public.js Show resolved Hide resolved
import { Box, Heading, Text } from '@primer/react';
import { DefaultLayout } from 'pages/interface/index.js';

export default function ConfirmRecoverPassword() {
Copy link
Contributor

Choose a reason for hiding this comment

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

o Google provavelmente vai indexar essa página /recuperar/confirmar, seria interessante ter um no-index aqui

Copy link
Owner Author

Choose a reason for hiding this comment

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

Excelente! Você sabe se é possível fazer isso pela página, ou só pelo robots.txt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Dá sim, é só colocar a metatag <meta name="robots" content="noindex">

import { Box, Heading, Link, Text } from '@primer/react';
import { DefaultLayout } from 'pages/interface/index.js';

export default function ConfirmRecoverPassword() {
Copy link
Contributor

Choose a reason for hiding this comment

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

mesma coisa aqui sobre a indexação do Google

Copy link
Owner Author

Choose a reason for hiding this comment

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

Repito aqui a dúvida sobre fazer isso pela página ou robots.txt 👍

@gabrielew
Copy link
Contributor

Dando continuidade ao PR #358 de @gabrielew onde iniciei fazendo o squash dos commits e agora vou levar a feature até ao final e seguindo as sugestões que apareceram na issue de controle #324

boa!

@vercel
Copy link

vercel bot commented May 24, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
tabnews ✅ Ready (Inspect) Visit Preview May 25, 2022 at 4:18PM (UTC)

@filipedeschamps filipedeschamps changed the title feat(recovery): implement the first draft Recuperação de Senha May 24, 2022
@filipedeschamps
Copy link
Owner Author

filipedeschamps commented May 24, 2022

Alterações feitas no último commit 41707e9

  1. O foco foi na parte de POST da implementação.
  2. Troquei endpoint e model de recover (que é uma ação) para recovery (que é um substantivo).
  3. Fiz implementação de recusar a request se usuário não possuir feature de Login (create:session), mas isso vai mais atrapalhar do que ajudar, pois ele pode não ter ativado a conta mas também ter se esquecido da senha. Então removi a implementação, mas é muito simples de fazer ela caso precise.
  4. Fiz os testes utilizando .toStrictEqual(), que inclusive pegou uma propriedade que não estava sendo testada em nenhum lugar:type. Essa bola foi levantada por @EmanoelCristhian e mandatório usar isso daqui pra frente.
  5. Implementei o Filtro de Entrada e Filtro de Saída dos dados.
  6. Optei por criar uma nova tabela específica para reset de senhas, pois comecei a pensar na outra tabela generalista e minha cabeça começou a driftar com novas condicionais e novos testes que não vão mudar em nada na vida de quem hoje está trancado por fora da sua conta. Podemos fazer essa tabela e refatoração mais para frente.
  7. É feito a validação do recebimento do email.

image

@gabrielew
Copy link
Contributor

Alterações feitas no último commit 41707e9

  1. O foco foi na parte de POST da implementação.
  2. Troquei endpoint e model de recover (que é uma ação) para recovery (que é um substantivo).
  3. Fiz implementação de recusar a request se usuário não possuir feature de Login (create:session), mas isso vai mais atrapalhar do que ajudar, pois ele pode não ter ativado a conta mas também ter se esquecido da senha. Então removi a implementação, mas é muito simples de fazer ela caso precise.
  4. Fiz os testes utilizando .toStrictEqual(), que inclusive pegou uma propriedade que não estava sendo testada em nenhum lugar:type. Essa bola foi levantada por @EmanoelCristhian e mandatório usar isso daqui pra frente.
  5. Implementei o Filtro de Entrada e Filtro de Saída dos dados.
  6. Optei por criar uma nova tabela específica para reset de senhas, pois comecei a pensar na outra tabela generalista e minha cabeça começou a driftar com novas condicionais e novos testes que não vão mudar em nada na vida de quem hoje está trancado por fora da sua conta. Podemos fazer essa tabela e refatoração mais para frente.
  7. É feito a validação do recebimento do email.
image

Só não entendi a parte 3. type

@filipedeschamps
Copy link
Owner Author

No passado, o type foi introduzido nas mensagens de erro da API para o client poder ter total refino sobre o que fazer com essa mensagem.

Isso foi usado aqui:

{errorObject?.type === 'string.alphanum' && (

E possibilitou colocar uma "dica" caso a pessoa escolha um nome de usuário que esbarre na regra dos alfanuméricos.

Então um erro comum é esse (de faltando o username):

image

Mas se a pessoa cair na regra do alfanumérico, podemos refinar com a dica que comentei acima:

image

E porque não fazer somente um parse do campo message? Porque ter um campo programático é muito mais estável de se criar lógicas em cima, tanto que agora podemos (e vamos) refatorar todas as mensagens de erro, mas a lógica nos clients continua inalterada.

@filipedeschamps
Copy link
Owner Author

Alterações feitas no último commit 6a4a56e

  1. O foco foi na parte de PATCH da implementação.
  2. Refatoração como no commit anterior sobre o POST, sem muitos mistérios.
  3. Adicionei um método de update().
  4. O model user tem uma trava temporária para não deixar atualizar "password" nem "email" (enquanto não temos dupla verificação). Eu removi isso do model e passei para o controller, pois isso liberou utilizar o model user para atualizar a senha do usuário dentro do model recovery. Mas isso modificou onde a validação acontece e eu modifiquei o teste que garante esse comportamento temporário.

image

@filipedeschamps
Copy link
Owner Author

Destaques do commit a6304a5

  1. Removi a necessidade das features relacionadas a recovery, porque para quem estava logado (e não possui essas features) e quer usar esse fluxo para trocar de senha, teria sido barrado.
  2. Dentro do model recovery, estou abraçando os erros de NotFoundError jogados pelo model user para serem tratados como ValidationError e unificar o tratamento de erro pelo frontend. Ficou muito mais simples e confiável.

Destaques do commit fcc11de

  1. Removi o util de verificação de email. Quanto menos "utils" tivermos melhor, e deixa o backend validar o email.
  2. Normalização dos erros de formulário para responses com 400, e mensagem global de erro >= 401
  3. Vou deixar a remoção da indexação das páginas de confirmação quando o PR do @gabrnunes entrar na main.
  4. Um cuidado de UX sobre o setIsLoading(false): não coloque ele no finally, pois no caso de uma resposta com sucesso (200 ou 201), o botão ficará disponível enquanto o usuário está carregando a próxima página. Então nesse caso de sucesso, o botão deverá continuar desabilitado.

@filipedeschamps
Copy link
Owner Author

Migration foi rodada em ambiente de Preview e nova feature já pode ser testada pela URL: https://tabnews-git-recovery-tabnews.vercel.app/

@filipedeschamps filipedeschamps merged commit 2fcd6e7 into main May 25, 2022
@filipedeschamps filipedeschamps deleted the recovery branch May 25, 2022 20:14
@filipedeschamps filipedeschamps mentioned this pull request May 25, 2022
3 tasks
@filipedeschamps
Copy link
Owner Author

Já está em Produção 🎉 Post comemorativo: https://www.tabnews.com.br/filipedeschamps/nova-melhoria-recuperacao-de-senha

Let's goooooo!!!

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.

None yet

3 participants