Somente exibir campo de Contato quando logado(staff ou user) #77
Conversation
| userId?: string, | ||
| ) { | ||
| if (!sessionId) return false; | ||
| if (!userId) return false; |
There was a problem hiding this comment.
esses dois ifs aqui ficaram redundantes, não?
porque se não tiver sessionId nem userId, a query abaixo já vai retornar false de qualquer jeito
There was a problem hiding this comment.
É só uma guard clause pra se não for informada nem gastar uma query com dado inválido. Posso remover se for o caso...
| @@ -35,9 +37,10 @@ export class ShelterController { | |||
| } | |||
|
|
|||
| @Get(':id') | |||
| async show(@Param('id') id: string) { | |||
| @UseGuards(ApplyUser) | |||
There was a problem hiding this comment.
aqui eu acho que não precisa do ApplyUser, pode usar direto @UseGuards(UserGuard) e aí ele vai aprovar todo mundo que tenha, no mínimo, a permissão de User, visto que é uma hierarquia de permissões
There was a problem hiding this comment.
O problema que usar o @UseGuards(UserGuard) ele vai aplicar a restrição pra rota toda em vez de apenas um atributo(o de telefone). Então usuários não logados não vão conseguir ver os detalhes do abrigo quebrando essa página.
As screens com o projeto rodando essa brach:
There was a problem hiding this comment.
aaah sim, te entendi. faz sentido.
pergunta: o nest.js não deveria ter um "ApplyUser" nativo não?
There was a problem hiding this comment.
não sei/consegui achar, se souber como eu altero já também
src/shelter/shelter.service.ts
Outdated
| async show(id: string) { | ||
| async show(id: string, user: any) { | ||
| const isLogged = | ||
| Boolean(user) && Boolean(user?.sessionId) && Boolean(user?.userId); |
There was a problem hiding this comment.
essa lógica aqui está correta.
porém, não acho que a camada de service precise receber o contexto do usuário que está logado.
por isso, essa lógica deveria ficar na camada de controller e aqui o método show poderia receber um boolean por parametro chamado shouldShowContact que por padrão é false.
assim a funcionalidade segue a mesma, mas mantemos a integridade da responsabilidade da service
There was a problem hiding this comment.
Sim, é uma boa, vou alterar
Já aproveitando o gancho: será que não seria interessante já seguir o padrão repository também pra puxar essas partes do banco e deixar a camada de service somente com as regras de negócio? até facilitaria o teste de services sem precisar mockar...
Descrição
É necessário ocultar o dado de contato(telefone) para o público em geral e restringir para apenas usuários(
STAFFeUSER) para controlar melhor quem visualiza e deixar apenas para pessoal capacitado(registrado).Solução
Como as guards atuais no
controller(como@UseGuards(UserGuard)) restrigem totalmente o acesso a aquela rota para usuários registrados foi precisoEntão passar para uma função para verificar se o usuário(caso esteja logado) seja um dos permitidos(no momento todos)Basta verificar se um usuário é passado no guards(verificar se recebe do contexto de guarduser,userIdesessionId)contactTestes
É preciso fazer uns testes já que modifica alguns pontos e pode quebrar outros. Apesar de testar localmente e não ter quebrado nada os casos recomendados(mais impactados) foram: