feat: add favorite/unfavorite of articles #148
Conversation
WalkthroughThe changes introduce functionality for users to favorite and unfavorite articles in the application. This is achieved by adding new endpoints to the articles API, corresponding service and repository methods to handle the logic, and updating the database interactions accordingly. The new endpoints require authentication and allow users to mark or unmark articles as favorites, updating the state in the database and returning the updated article information. No functional changes were made to the main server setup. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API (articles.plugin)
participant Service
participant Repository
participant Database
User->>API (articles.plugin): POST /:slug/favorite (with token)
API (articles.plugin)->>Service: favoriteArticle(slug, userId)
Service->>Repository: favoriteArticle(slug, userId)
Repository->>Database: Insert favorite, fetch article
Database-->>Repository: Article data
Repository-->>Service: Article data
Service-->>API (articles.plugin): Article response
API (articles.plugin)-->>User: Article response
User->>API (articles.plugin): DELETE /:slug/favorite (with token)
API (articles.plugin)->>Service: unfavoriteArticle(slug, userId)
Service->>Repository: unfavoriteArticle(slug, userId)
Repository->>Database: Delete favorite, fetch article
Database-->>Repository: Article data
Repository-->>Service: Article data
Service-->>API (articles.plugin): Article response
API (articles.plugin)-->>User: Article response
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/articles/articles.service.ts (2)
122-138: LGTM, but consider handling state more consistentlyThe implementation for favoriting an article is well-structured, but there's a potential consistency issue. The method generates the response from the database state and then manually modifies it. Consider refetching the article after favoriting to ensure the response reflects the actual database state.
async favoriteArticle(slug: string, currentUserId: number) { const article = await this.repository.favoriteArticle(slug, currentUserId); const baseResponse = await this.generateArticleResponse(article, currentUserId); // If the article is already favorited, return the current state if (baseResponse.article.favorited) { return baseResponse; } - return { - article: { - ...baseResponse.article, - favorited: true, - favoritesCount: baseResponse.article.favoritesCount + 1, - }, - }; + // Refetch to get the updated state + const updatedArticle = await this.repository.findBySlug(slug); + return this.generateArticleResponse(updatedArticle!, currentUserId); }
140-156: Same refactoring suggestion as aboveSimilar to the favorite method, consider refetching the article to ensure the response reflects the actual database state rather than manually adjusting the values.
async unfavoriteArticle(slug: string, currentUserId: number) { const article = await this.repository.unfavoriteArticle(slug, currentUserId); const baseResponse = await this.generateArticleResponse(article, currentUserId); // If the article is not favorited, return the current state if (!baseResponse.article.favorited) { return baseResponse; } - return { - article: { - ...baseResponse.article, - favorited: false, - favoritesCount: baseResponse.article.favoritesCount - 1, - }, - }; + // Refetch to get the updated state + const updatedArticle = await this.repository.findBySlug(slug); + return this.generateArticleResponse(updatedArticle!, currentUserId); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.gitignore(1 hunks)src/articles/articles.plugin.ts(1 hunks)src/articles/articles.repository.ts(2 hunks)src/articles/articles.service.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
src/articles/articles.repository.ts (1)
src/articles/articles.model.ts (1) (1)
favoriteArticles(39-52)
src/articles/articles.plugin.ts (1)
src/articles/articles.schema.ts (1) (1)
ReturnedArticleResponseSchema(57-59)
🔇 Additional comments (6)
.gitignore (1)
47-47: LGTM!Adding
.qodoto the gitignore file is a straightforward change that prevents these files from being committed to the repository.src/articles/articles.plugin.ts (2)
117-130: LGTM!The POST endpoint for favoriting an article is well-implemented. It properly requires user authentication, extracts the necessary parameters, and follows the established pattern for error handling and response structure.
132-145: LGTM!The DELETE endpoint for unfavoriting an article is properly implemented, requiring authentication and following the same pattern as other endpoints.
src/articles/articles.repository.ts (3)
9-10: LGTM!The import of
NotFoundErrorfrom 'elysia' is correctly added and is used in the new methods for error handling.
201-228: LGTM!The
favoriteArticlemethod is well-implemented with proper error handling and duplicate prevention. It checks if the article exists and ensures a user doesn't favorite the same article multiple times.
230-261: LGTM!The
unfavoriteArticlemethod is well-implemented with proper error handling and edge case consideration. It checks if the article exists and if the user has already favorited it before attempting to remove the favorite.
aruaycodes
changed the title
yamcodes
left a comment
yamcodes
left a comment
There was a problem hiding this comment.
The repository logic needs to be fixed to handle favoriting and returning the updated state, the service level shouldn't be responsible to mask the issues of the repository level
|
Live testing shows that the operation returns outdated information - favoriting an article shows a favorite count of 0 and only once favoriting again it shows the updated state. Same applies for unfavoriting. I can see that the operation works on the database level but the response is not updated. I would recommended solving this at the repository level - meaning, the database call to favorite/unfavorite should already return the correct updated state (and not the one before the operation was done) |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignore(1 hunks)src/articles/articles.repository.ts(1 hunks)src/main.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- src/articles/articles.repository.ts
yamcodes
left a comment
yamcodes
left a comment
There was a problem hiding this comment.
Good job, according to my testing the PR gets the job done.
There are some things that are unnecessary and some improvements we can make
| async unfavoriteArticle(slug: string, currentUserId: number) { | ||
| const article = await this.findBySlug(slug); | ||
| if (!article) { | ||
| return null; | ||
| } | ||
|
|
||
| // Delete the favorite and get the updated article state | ||
| await this.db | ||
| .delete(favoriteArticles) | ||
| .where( | ||
| and( | ||
| eq(favoriteArticles.articleId, article.id), | ||
| eq(favoriteArticles.userId, currentUserId), | ||
| ), | ||
| ); | ||
|
|
||
| // Return the updated article state | ||
| return this.findBySlug(slug); | ||
| } |
There was a problem hiding this comment.
Overall there are 1 or 3 db operations (depending if the article is found, or not)
I believe we can reduce this to 2 and even 1 op total.
There was a problem hiding this comment.
@yamcodes You're absolutely right about the multiple DB operations. The current implementation has up to 3 database operations:
- Initial
findBySlug - Insert/delete operation
- Second
findBySlugto get updated state
Here are two possible optimizations:
Option 1: Reduce to 2 operations
Keep the initial check but don't fetch the article twice:
async favoriteArticle(slug: string, currentUserId: number) {
const article = await this.findBySlug(slug);
if (!article) return null;
await this.db
.insert(favoriteArticles)
.values({ articleId: article.id, userId: currentUserId })
.onConflictDoNothing();
// Instead of calling findBySlug again, update the article object
const favorited = true;
const favoritesCount = (article.favoritedBy?.length || 0) + 1;
return {
...article,
favoritedBy: [...(article.favoritedBy || []), { userId: currentUserId }],
};
}Option 2: Reduce to 1 operation (more advanced)
Use a transaction with a CTE to handle everything in one query:
async favoriteArticle(slug: string, currentUserId: number) {
// Single operation that handles existence check, favorite action, and returns data
const result = await this.db.transaction(async (tx) =
> {
const articleResult = await tx.query.articles.findFirst({
where: eq(articles.slug, slug)
});
if (!articleResult) return null;
await tx
.insert(favoriteArticles)
.values({ articleId: articleResult.id, userId: currentUserId })
.onConflictDoNothing();
return tx.query.articles.findFirst({
where: eq(articles.slug, slug),
with: {
author: {
with: { followers: true }
},
favoritedBy: true
}
});
});
return result;
}Which approach would you prefer to implement?
Co-authored-by: Yam Borodetsky <yam@yam.codes>
Co-authored-by: Yam Borodetsky <yam@yam.codes>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Description
PR Checklist
bun docsSummary by CodeRabbit