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

PR de correção #15

Open
wants to merge 33 commits into
base: correcao-projeto
Choose a base branch
from
Open

PR de correção #15

wants to merge 33 commits into from

Conversation

jvpalves
Copy link

Avaliando o projeto de vocês!

OsmanRodrigues and others added 30 commits July 20, 2020 16:44
instalação das dependencias e modulos iniciais
ajustes na organização das dependencias
implementação do endpoit getpostbyid
método de signup feito e de login á fazer.
WellDMLT and others added 2 commits July 24, 2020 16:26
implementação dos endpoints create e delete friendship
Copy link
Author

@jvpalves jvpalves left a comment

Choose a reason for hiding this comment

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

Oi, gente! Eu adorei o projeto de vocês! Tudo funciona super bem! Os principais pontos de atenção dizem respeito a clean code. Vocês acabaram misturando um pouco os nomes de variáveis entre os endpoints, o que acabou gerando algumas confusões nos meus testes haha. Gostei muito da organização do código, tudo muito fácil de ler e navegar. Achar probleminhas e corrigir também foi fácil, e enxergar as limitações com o TODO também foi bacana (apesar de eu sempre apontar comentários haha). Vocês parecem ter sacado bem os conceitos da semana. Parabéns!
Fiz uns testes envolvendo The Office no banco, caso queiram dar uma olhada depois, e eu fiquei bem assim usando a aplicação do grupo:

michael

create_at?: string,
description?:string
){
//TODO: validar criação de post mediante access token
Copy link
Author

Choose a reason for hiding this comment

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

Lembrem de remover os comentários depois!

Comment on lines +14 to +18
if(! img_url || img_url.trim() === ''){
throw new CustomError(416,'Missing post image url.');
}else if(! img_url.includes('http')){
throw new CustomError(406,'Invalid post image url.');
};
Copy link
Author

Choose a reason for hiding this comment

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

Gostei da validação aqui!

Comment on lines +22 to +31

await usePostsDb.createPost({
id: idGen.generate(),
img_url,
type,
create_at,
description
});
};

Copy link
Author

Choose a reason for hiding this comment

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

O endpoint realmente acabou por criar o post sem gravar o autor do post

Copy link
Collaborator

Choose a reason for hiding this comment

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

Macacos me mordam! Eu lembrei de ter arrumado o campo na tabela pra receber essa info, mas esqueci de mudar na query! haha

Comment on lines +31 to +41

const requestInfo = await userBusiness.login(body.email, body.password);


if(requestInfo){
res.status(200).send({accessToken: requestInfo })
}

} catch (error) {
res.status(400).send({ error: error.message });
}
Copy link
Author

Choose a reason for hiding this comment

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

Temos um probleminha aqui. Se a senha estiver incorreta nunca caímos no catch e o endpoint carrega para sempre

try{
const body = req.body;
await new PostsBusiness().createPost(
body.img_url, body.type, body.create_at, body.description
Copy link
Author

Choose a reason for hiding this comment

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

Uma possibilidade aqui é deixar a criação das datas automática!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Eu passei por precaução pq tinha uma validação no business e um default pra o campo de data lá no banco atribuir a curdate caso não fosse passada uma data. É melhor prevenir do que remediar! haha

};
}

public async showFeed(id: string): Promise<any> {
Copy link
Author

Choose a reason for hiding this comment

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

Podia devolver Posts modelados aqui!

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.

4 participants