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 para Correção #9

Open
wants to merge 27 commits into
base: branch-para-correcao
Choose a base branch
from
Open

PR para Correção #9

wants to merge 27 commits into from

Conversation

joaogolias
Copy link
Contributor

Veja os comentários!

Meira-JH and others added 27 commits June 17, 2020 14:48
estrutura da aplicação. Data, Routes, Services e index.ts
follow , unfollow & getOhterProfile
editRecipe finalizado e funcionando
deleteRecipe finalizado e funcionando
Copy link
Contributor Author

@joaogolias joaogolias left a comment

Choose a reason for hiding this comment

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

Muito bom pessoal! O resultado final ficou excelente. Coloquei alguns comentários aí, mas, de verdade, fiquei impressionado!
Continuem assim!

@@ -0,0 +1,106 @@
.env

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Coloque a pasta build no .gitignore

.status(error.code)
.send({ message: error.message })
} else {
console.log(error)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Retire console.log desnecessários!

@@ -0,0 +1,20 @@
import {Request, Response, NextFunction} from 'express'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Amei!


const isEmail = validateEmail(email);
if (!isEmail) {
throw new CustomError("Email inválido.", 412);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

O código de erros poderia ser um ENUM para ficar mais centralizado

}
await new RecipeDataBase().deleteRecipe(recipeId);

response.status(200).send({ message: "Receita deletada!" });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Acho que esse retorno "Receita deletada". Se foi bem sucedido, já fica claro que foi deletado!

@@ -0,0 +1,77 @@
import { IdGenerator } from "../services/IdGenetor";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Muito muito bom!

Comment on lines +97 to +106
for (let i = 0; i < followingUsers.length; i++) {
const [user, userRecipes] = await Promise.all([
userDatabase.getUserById(followingUsers[i].id_following),
recipeDatabase.getRecipesByUserId(followingUsers[i].id_following),
]);

userRecipes.forEach((recipe: Object) =>
recipes.push({ ...recipe, userName: user.name })
);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gostei do jeito que vocês resolveram, mas o jeito mais correto seria fazer uma query só no banco, usando os JOIN para pegar as informações das receitas e dos usuários

);
}

recipes.sort((b, a) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Já daria para ordenar na query que falei ai em cima

Comment on lines +131 to +134
await new RefreshTokenDataBase().deleteUserRefreshToken(id);
await new FollowDatabase().deleteAllUserRelations(id);
await new RecipeDataBase().deleteAllRecipesFromUser(id);
await userDatabase.deleteUser(id);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lindo! Um Promise.all poderia ter sido usado aqui para deletar o refresh token as relações e as receitas ao mesmo tempo!

@@ -0,0 +1,7 @@

export default function validateEmail( email : string ) : boolean {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

MUITO BOM!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mas recomendo vocês irem atrás de libs de validação, como a class validator, que já vem com algumas coisas prontas!

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