From 50264f0b6616ba660b13343462b52247a8869d63 Mon Sep 17 00:00:00 2001 From: ndu Date: Wed, 11 Dec 2024 15:26:44 +0100 Subject: [PATCH] Revert "chore(fix): fix related articles logic" --- server/apps/research/admin/article_admin.py | 34 ++++------ server/apps/research/models/article.py | 63 ++++--------------- .../serializers/article_serializer.py | 19 ++---- 3 files changed, 32 insertions(+), 84 deletions(-) diff --git a/server/apps/research/admin/article_admin.py b/server/apps/research/admin/article_admin.py index 689ec34..e4b13fa 100644 --- a/server/apps/research/admin/article_admin.py +++ b/server/apps/research/admin/article_admin.py @@ -18,23 +18,24 @@ def formfield_for_foreignkey(self, db_field, request, **kwargs): if db_field.name == 'to_article': # Get the parent object (Article) from the request obj_id = request.resolver_match.kwargs.get('object_id') - # For new articles (when obj_id is None), show all ready articles - base_queryset = Article.objects.filter(status='ready') + parent_obj = None if obj_id: try: parent_obj = Article.objects.get(pk=obj_id) - # Exclude self-reference and articles that already have a relationship - base_queryset = base_queryset.exclude( - id=parent_obj.id - ).exclude( - related_from__to_article=parent_obj - ) except Article.DoesNotExist: pass + base_queryset = Article.objects.filter(status='ready') + if parent_obj: + # Exclude self-reference and articles that already have a relationship + base_queryset = base_queryset.exclude( + id=parent_obj.id + ).exclude( + related_from__to_article=parent_obj + ) kwargs['queryset'] = base_queryset - return super().formfield_for_foreignkey(db_field, request, **kwargs) + return super().formfield_for_foreignkey(db_field, request, **kwargs) class ArticleForm(forms.ModelForm): class Meta: model = Article @@ -55,18 +56,9 @@ def current_slug_history(self, obj): current_slug_history.short_description = 'Slug Change History' def get_inlines(self, request, obj): - # Allow inlines for both new and existing articles - return [RelatedArticleInline] - - def save_related(self, request, form, formsets, change): - """Handle saving related articles for both new and existing articles.""" - super().save_related(request, form, formsets, change) - - # Process related articles from inline formsets - for formset in formsets: - if isinstance(formset, RelatedArticleInline): - # The related articles will be saved automatically through the formset - pass + if obj is None: + return [] + return super().get_inlines(request, obj) fieldsets = [ ('Article Details', {'fields': ['title', 'slug', 'authors', 'acknowledgement', 'categories', 'thumb', 'content', 'summary', 'status', 'scheduled_publish_time']}), diff --git a/server/apps/research/models/article.py b/server/apps/research/models/article.py index 7f33860..765a2be 100644 --- a/server/apps/research/models/article.py +++ b/server/apps/research/models/article.py @@ -119,14 +119,6 @@ def build_table_of_contents(self): def save(self, *args, **kwargs): """Override the save method to generate a unique slug and build table of contents.""" - is_new = self.pk is None - temp_related_articles = [] - - # If this is a new article and there are related articles in the form - if is_new and hasattr(self, "_temp_related_articles"): - temp_related_articles = self._temp_related_articles - delattr(self, "_temp_related_articles") - if not self.slug or self.title_update(): self.slug = self.generate_unique_slug() @@ -158,24 +150,7 @@ def save(self, *args, **kwargs): ): self.status = "ready" - with transaction.atomic(): - super().save(*args, **kwargs) - - # If this was a new article and we had temporary related articles - if is_new and temp_related_articles: - for related_article in temp_related_articles: - RelatedArticle.objects.create( - from_article=self, to_article=related_article - ) - - def set_temp_related_articles(self, related_articles): - """ - Store related articles temporarily before the initial save. - - Args: - related_articles: List of Article instances to be related - """ - self._temp_related_articles = related_articles + super().save(*args, **kwargs) def generate_unique_slug(self): """Generate a unique slug for the article.""" @@ -199,7 +174,7 @@ def title_update(self): class RelatedArticle(models.Model): """Through model for related articles to prevent circular references.""" - + from_article = models.ForeignKey( Article, on_delete=models.CASCADE, related_name="related_from" ) @@ -216,35 +191,23 @@ class Meta: name="prevent_self_reference", ) ] - + def clean(self): # Prevent direct circular references - if ( - self.from_article.pk - and RelatedArticle.objects.filter( - from_article=self.to_article, to_article=self.from_article - ).exists() - ): + if RelatedArticle.objects.filter( + from_article=self.to_article, to_article=self.from_article + ).exists(): raise ValidationError("Circular references detected.") - + # Maximum of 3 related articles - if ( - self.from_article.pk - and RelatedArticle.objects.filter(from_article=self.from_article).count() - >= 3 - ): + if RelatedArticle.objects.filter(from_article=self.from_article).count() >= 3: raise ValidationError("Maximum of 3 related articles allowed.") - + def save(self, *args, **kwargs): - with transaction.atomic(): - # Only acquire lock if from_article exists in database - if self.from_article.pk: - RelatedArticle.objects.select_for_update().filter( - from_article=self.from_article - ).exists() - self.clean() - super().save(*args, **kwargs) - + # Acquire a lock on related articles for this from_article + RelatedArticle.objects.select_for_update().filter(from_article=self.from_article) + self.clean() + super().save(*args, **kwargs) class ArticleSlugHistory(models.Model): """Model to track historical slugs for articles.""" diff --git a/server/apps/research/serializers/article_serializer.py b/server/apps/research/serializers/article_serializer.py index 451d60c..fd7373a 100644 --- a/server/apps/research/serializers/article_serializer.py +++ b/server/apps/research/serializers/article_serializer.py @@ -53,16 +53,10 @@ class Meta: model = Article fields = ['title', 'slug', 'categories', 'thumb', 'content', 'summary', 'acknowledgement', 'status', 'authors', 'scheduled_publish_time', 'is_sponsored', 'sponsor_color', 'sponsor_text_color', 'related_article_ids'] - def validate_related_article_ids(self, value): - """Validate related articles.""" + def validate_related_articles_ids(self, value): + """Validate related articles""" if len(value) > 3: raise serializers.ValidationError("You can only have a maximum of 3 related articles.") - - # Check for self-reference - instance = self.instance - if instance and instance.id in [article.id for article in value]: - raise serializers.ValidationError("An article cannot be related to itself.") - return value def create(self, validated_data: dict) -> Article: @@ -78,19 +72,18 @@ def create(self, validated_data: dict) -> Article: if user_author: authors = [user_author] - # Create and save the article first - article = Article.objects.create(**validated_data) + article = Article(**validated_data) + article.save() if authors: article.authors.set(authors) if categories: article.categories.set(categories) - # Handle related articles after the article is created + # Handle related articles for related_article in related_article_ids: RelatedArticle.objects.create( - from_article=article, - to_article=related_article + from_article=article, to_article=related_article ) return article