-
Notifications
You must be signed in to change notification settings - Fork 3
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
chore(fix): fix related articles logic #199
Conversation
WalkthroughThe pull request introduces significant modifications to the handling of related articles in the admin interface and models. Key changes include updates to the Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (6)server/apps/research/models/article.py (6)
The implementation for handling temporary related articles when saving a new article is correctly implemented. The checks for
The use of
The method
The
The validation correctly limits the number of related articles to a maximum of 3, preventing over-association and ensuring that each article maintains a manageable set of related articles.
By evaluating the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
server/apps/research/serializers/article_serializer.py (2)
82-93
: Useset_temp_related_articles
method for consistent handlingIn the
create
method, related articles are being created directly after the article is saved. Since theArticle
model now supports setting temporary related articles via theset_temp_related_articles
method, consider using this method to leverage the model's logic and maintain consistency.Update the
create
method to set temporary related articles before saving:# Create the article instance without saving article = Article(**validated_data) # Set temporary related articles if related_article_ids: article.set_temp_related_articles(related_article_ids) # Save the article, which will handle related articles in the `save` method article.save() # Handle authors and categories after saving if authors: article.authors.set(authors) if categories: article.categories.set(categories)This approach utilizes the
Article
model'ssave
method to handle related articles, ensuring consistency and encapsulating the logic within the model.
Line range hint
114-123
: Consider updating self-reference validation during creationIn the
validate_related_article_ids
method, the self-reference check usesinstance.id
, which isNone
during creation. This means self-references might not be caught when creating a new article.Modify the validation to handle the creation scenario:
# Check for self-reference if self.instance: article_id = self.instance.id else: # If creating, article_id can be derived from validated data or set to None article_id = None if article_id in [article.id for article in value]: raise serializers.ValidationError("An article cannot be related to itself.")Or, since the article doesn't have an ID yet during creation, you can prevent including the article's own ID in
related_article_ids
.server/apps/research/admin/article_admin.py (1)
61-69
: Remove unnecessary override ofsave_related
methodIn
ArticleAdmin
, thesave_related
method is overridden but does not add any additional functionality beyond callingsuper().save_related
. The loop over formsets does not perform any operations.Consider removing the
save_related
method override to simplify the code unless you plan to add custom processing for related articles.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
server/apps/research/admin/article_admin.py
(2 hunks)server/apps/research/models/article.py
(4 hunks)server/apps/research/serializers/article_serializer.py
(2 hunks)
Summary by CodeRabbit
New Features
Bug Fixes
Documentation