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

Fix - Redirection to Login if Response Status is 302 #517

Merged
merged 10 commits into from
Aug 19, 2021

Conversation

LuanEdCosta
Copy link
Contributor

@LuanEdCosta LuanEdCosta commented Aug 16, 2021

  • Create AuthExpired interceptor inside the services folder
  • Attach AuthExpired interceptor in all API instances
  • Make all files inside the services folder return the axios instance
  • Test if all axios instances have the AuthExpired interceptor
  • Create factory to generate a new axios instance with the AuthExpired interceptor attached. This factory adds a identifier to the instance, so it's possible to check later if a instance was created with the factory.

P.S. Attach the interceptor to the axios directly is a bad practice because in the future we may want to create an instance axios that does not redirect to the login if the response status be 302.

@LuanEdCosta LuanEdCosta requested a review from fberanizo August 16, 2021 21:15
@github-actions
Copy link

Dicas para revisão de código

Commits

  • Título (1a linha do commit): apresentar resumo do que foi alterado/adicionado/removido.
    ex: adiciona action que salva parametros no backend; exibe rótulo no componente de selecao de dataset;
  • Descrição (outras linhas): dar mais detalhes de cada alteração:
    • motivos das alterações
      ex: havia um bug que causava...; nova funcionalidade que faz isso...; código foi movido para...;
    • bibliotecas adicionadas e versões (package.json)
      ex: atualiza para antd v4.6;
    • testes unitários criados/alterados
      ex: adiciona testes para a action fetchProjectSuccess;
  • Mensagens auto-explicativas! Quem revisa o código deve entender o que foi feito (e porque foi feito) sem perguntar para quem fez o commit.
  • Não devem ter conflitos. Solicitar que sejam resolvidas as ocorrências de "This branch has conflicts that must be resolved".

SonarCloud Quality Gate

  • Coverage > 80.0%, e sempre que possível = 100%
  • 0 Bugs, 0 Code Smells, 0 Vulnerabilities

Build Github actions COM SUCESSO

ReactJS

  • Usar Node 12 ou 14.
  • Remover console.log.
  • Não deixar código-fonte comentado.
  • Pasta de módulos devem ser escritas em lower case
  • Pasta de componentes devem ser escritas em camel case
  • Nunca utilizar spread em retorno de reducer sem alteração
  • Sempre utilizar hooks ao invés de HOCs
  • Itens em uma lista devem ser renderizados com uma chave única (preferir uuid)
  • Comentar apenas códigos que não estão muito claros
  • Não utilizar comentários para blocos
  • Action types devem conter @

Redux

As instruções a seguir foram retiradas do Redux Style Guide.

@LuanEdCosta
Copy link
Contributor Author

@fberanizo Teria como subir o código desta branch em um ambiente com login para testarmos se o que fiz funciona? Acredito que funciona sim porque simulei em um com login, mas nada como testar de verdade. 👍

Copy link
Member

@fberanizo fberanizo left a comment

Choose a reason for hiding this comment

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

Minha percepção é que adicionando o interceptor em cada service fica mais fácil de entender, porém mais verboso (com código duplicado).

Se tivesse uma função "fábrica" pra substituir o axios.create, e que já adicionasse o interceptor, acho ficaria mais enxuto o código. Essa sugestão tem uma desvantagem de tornar o desenvolvimento de futuros services anti-natural, principalmente pra novos devs (não se usaria o axios diretamente).

Bem...tem prós e contras.
@schafferjrdev como você prefere?

@LuanEdCosta
Copy link
Contributor Author

Agora está funcionando e consegui criar a factory com as tipagens do axios. @fberanizo Já pode testar no awsplatiagro25 porque já implantei lá. Só uma coisa que achei não tão legal: o redirect demora um pouco, então as mensagens do antd ainda aparecem.

@fberanizo
Copy link
Member

Agora está funcionando e consegui criar a factory com as tipagens do axios. @fberanizo Já pode testar no awsplatiagro25 porque já implantei lá. Só uma coisa que achei não tão legal: o redirect demora um pouco, então as mensagens do antd ainda aparecem.

Acho que não tem muito problema do jeito que ficou.
Será que dá pra transformar o resultado da request em uma exceção com mensagem de erro mais amigável?
Talvez a gente possa fazer um "cancel" na requisição para garantir que no service chegue exception.
https://axios-http.com/docs/cancellation

O que você acha @LuanEdCosta ?

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

64.4% 64.4% Coverage
0.0% 0.0% Duplication

Copy link
Member

@fberanizo fberanizo left a comment

Choose a reason for hiding this comment

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

👍

@fberanizo fberanizo merged commit 344471d into master Aug 19, 2021
@LuanEdCosta LuanEdCosta deleted the fix/redirection-to-login branch February 22, 2022 13:15
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.

3 participants