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

Develop #48

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Pavlo-Petrashevskyi
Copy link

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Great job on your code! It's well-structured and organized. There are a few minor points for improvement, such as specifying the expected return type for functions, but they don't impact the functionality of your code. Keep up the good work! 👍

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

src/api/api.ts Outdated
Comment on lines 8 to 14
export const getUserByEmail = (email: string) => {
return client.get('/users', {
params: {
email,
}
})
}

Choose a reason for hiding this comment

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

It's a good practice to specify the expected return type for each function. This will make your code more predictable and easier to debug. For example, you could specify that this function returns a Promise that resolves to an AxiosResponse.

Choose a reason for hiding this comment

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

done

src/api/api.ts Outdated
Comment on lines 16 to 21
export const createUser = (email: string, name: string) => {
return client.post('/users', {
email,
name,
})
}

Choose a reason for hiding this comment

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

Again, it's recommended to specify the expected return type for this function. This will help to prevent potential errors in the future.

Choose a reason for hiding this comment

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

done

src/api/api.ts Outdated
Comment on lines 25 to 31
export const getPostsById = (userId: number) => {
return client.get('/posts', {
params: {
userId,
}
})
}

Choose a reason for hiding this comment

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

The function name getPostsById suggests that it should return a single post by its id, but it seems to return multiple posts by user id. Consider renaming it to getPostsByUserId for better clarity.

Choose a reason for hiding this comment

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

done

src/api/api.ts Outdated
Comment on lines 33 to 39
export const createPost = (title: string, body: string, userId: number) => {
return client.post('/posts', {
title,
body,
userId,
})
}

Choose a reason for hiding this comment

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

Specify the expected return type for this function to improve code predictability and debuggability.

Choose a reason for hiding this comment

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

done

src/api/api.ts Outdated
Comment on lines 41 to 47
export const updatePost = (title: string, body: string, postId: number, userId: number) => {
return client.patch(`/posts/${postId}`, {
title,
body,
userId,
})
}

Choose a reason for hiding this comment

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

Specify the expected return type for this function to improve code predictability and debuggability.

Choose a reason for hiding this comment

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

done

src/api/api.ts Outdated
Comment on lines 49 to 51
export const deletePost = (postId: number) => {
return client.delete(`/posts/${postId}`)
}

Choose a reason for hiding this comment

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

Specify the expected return type for this function to improve code predictability and debuggability.

Choose a reason for hiding this comment

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

done

src/api/api.ts Outdated
Comment on lines 55 to 61
export const getCommentsByPostId = (postId: number) => {
return client.get('/comments', {
params: {
postId,
}
})
}

Choose a reason for hiding this comment

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

Specify the expected return type for this function to improve code predictability and debuggability.

Choose a reason for hiding this comment

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

done

src/api/api.ts Outdated
Comment on lines 63 to 70
export const createComment = (postId: number, name: string, body: string, email: string) => {
return client.post('/comments', {
postId,
name,
body,
email,
})
}

Choose a reason for hiding this comment

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

Specify the expected return type for this function to improve code predictability and debuggability.

Choose a reason for hiding this comment

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

done

src/api/api.ts Outdated
Comment on lines 72 to 74
export const deleteComment = (commentId: number) => {
return client.delete(`/comments/${commentId}`)
}

Choose a reason for hiding this comment

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

Specify the expected return type for this function to improve code predictability and debuggability.

Choose a reason for hiding this comment

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

done

…; changing api function names for better readability
Copy link

@volodymyr-soltys97 volodymyr-soltys97 left a comment

Choose a reason for hiding this comment

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

Great! 🔥

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