From f059eba9e3125b02fac5bf409a8a0fc93bf0c309 Mon Sep 17 00:00:00 2001 From: ndu Date: Thu, 12 Dec 2024 08:07:21 +0100 Subject: [PATCH] chore(fix): add fixes --- .../serializers/article_serializer.py | 180 ++++++++---------- 1 file changed, 76 insertions(+), 104 deletions(-) diff --git a/server/apps/research/serializers/article_serializer.py b/server/apps/research/serializers/article_serializer.py index 5ae0b69..95e85ed 100644 --- a/server/apps/research/serializers/article_serializer.py +++ b/server/apps/research/serializers/article_serializer.py @@ -1,9 +1,13 @@ from rest_framework import serializers +from django.db import transaction +import logging from django.core.exceptions import ValidationError as DjangoValidationError from ..models import Article, Author, Category, RelatedArticle from .author_serializer import AuthorSerializer from .category_serializer import CategorySerializer +logger = logging.getLogger(__name__) + class ArticleListSerializer(serializers.ModelSerializer): categories = CategorySerializer(many=True) authors = AuthorSerializer(many=True) @@ -21,70 +25,81 @@ class RelatedArticleSerializer(serializers.ModelSerializer): class Meta: model = RelatedArticle fields = ['id', 'title', 'slug', 'thumb'] + class ArticleSerializer(serializers.ModelSerializer): + def _handle_error(self, error, operation_type, context_data): + """ + Centralized error handling for article operations + """ + if isinstance(error, DjangoValidationError): + raise serializers.ValidationError(error.message_dict) from error + + logger.error( + f"Error {operation_type} article", + extra={**context_data, "error": str(error)}, + exc_info=True + ) + raise serializers.ValidationError({ + "non_field_errors": [f"Unable to {operation_type} article. Please try again later."] + }) from error + def create(self, validated_data): try: - # Your existing creation logic here return super().create(validated_data) - except DjangoValidationError as e: - # Handle validation errors specifically, these are safe to expose - raise serializers.ValidationError(e.message_dict) except Exception as e: - # Log the detailed error for debugging - logger.error( - "Error creating article", - extra={ - "validated_data": validated_data, - "error": str(e) - }, - exc_info=True - ) - # Return a generic error message to the client - raise serializers.ValidationError({ - "non_field_errors": ["Unable to create article. Please try again later."] - }) + self._handle_error(e, "creating", {"validated_data": validated_data}) def update(self, instance, validated_data): try: - # Your existing update logic here return super().update(instance, validated_data) - except DjangoValidationError as e: - # Handle validation errors specifically - raise serializers.ValidationError(e.message_dict) except Exception as e: - # Log the detailed error for debugging - logger.error( - "Error updating article", - extra={ - "instance_id": instance.id if instance else None, - "validated_data": validated_data, - "error": str(e) - }, - exc_info=True - ) - # Return a generic error message to the client - raise serializers.ValidationError({ - "non_field_errors": ["Unable to update article. Please try again later."] + self._handle_error(e, "updating", { + "instance_id": instance.id if instance else None, + "validated_data": validated_data }) class ArticleCreateUpdateSerializer(serializers.ModelSerializer): """Serializer for creating and updating articles.""" authors = serializers.PrimaryKeyRelatedField(queryset=Author.objects.all(), many=True, required=False) categories = serializers.PrimaryKeyRelatedField(queryset=Category.objects.all(), many=True, required=False) - related_article_ids = serializers.PrimaryKeyRelatedField(queryset=Article.objects.filter(status='ready'), many=True, required=False, write_only=True) + related_article_ids = serializers.PrimaryKeyRelatedField( + queryset=Article.objects.filter(status='ready'), + many=True, + required=False, + write_only=True + ) 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'] - + fields = [ + 'title', 'slug', 'categories', 'thumb', 'content', 'summary', + 'acknowledgement', 'status', 'authors', 'scheduled_publish_time', + 'is_sponsored', 'sponsor_color', 'sponsor_text_color', 'related_article_ids' + ] + + def _handle_error(self, error, operation_type, context_data): + """ + Centralized error handling for article operations + """ + if isinstance(error, DjangoValidationError): + raise serializers.ValidationError(error.message_dict) from error + + logger.error( + f"Error {operation_type} article", + extra={**context_data, "error": str(error)}, + exc_info=True + ) + raise serializers.ValidationError({ + "non_field_errors": [f"Unable to {operation_type} article. Please try again later."] + }) from error + def validate_related_article_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 article_id = self.instance.id if self.instance else None - if article_id is None: # During creation, we check the request data for article ID + if article_id is None: request = self.context.get('request') if request and hasattr(request, 'data'): article_id = request.data.get('id') @@ -94,55 +109,40 @@ def validate_related_article_ids(self, value): return value + def _handle_relations(self, article, authors, categories, related_article_ids): + """Handle setting related objects for an article.""" + if authors: + article.authors.set(authors) + if categories: + article.categories.set(categories) + + if related_article_ids is not None: + with transaction.atomic(): + RelatedArticle.objects.filter(from_article=article).delete() + RelatedArticle.objects.bulk_create([ + RelatedArticle(from_article=article, to_article=related_article) + for related_article in related_article_ids + ]) + def create(self, validated_data): """Create a new Article instance.""" - request = self.context.get('request') authors = validated_data.pop('authors', []) categories = validated_data.pop('categories', []) related_article_ids = validated_data.pop('related_article_ids', []) try: - # Assign default author if none provided + request = self.context.get('request') if not authors and request and hasattr(request, 'user'): user_author = Author.objects.filter(user=request.user).first() if user_author: authors = [user_author] - # Create the article instance article = Article.objects.create(**validated_data) - - # Associate authors and categories - if authors: - article.authors.set(authors) - if categories: - article.categories.set(categories) - - # Bulk create related articles - related_articles = [ - RelatedArticle(from_article=article, to_article=related_article) - for related_article in related_article_ids - ] - RelatedArticle.objects.bulk_create(related_articles) - + self._handle_relations(article, authors, categories, related_article_ids) return article - except DjangoValidationError as e: - # Handle validation errors specifically, these are safe to expose - raise serializers.ValidationError(e.message_dict) except Exception as e: - # Log the detailed error for debugging - logger.error( - "Error creating article", - extra={ - "validated_data": validated_data, - "error": str(e) - }, - exc_info=True - ) - # Return a generic error message to the client - raise serializers.ValidationError({ - "non_field_errors": ["Unable to create article. Please try again later."] - }) + self._handle_error(e, "creating", {"validated_data": validated_data}) def update(self, instance: Article, validated_data: dict) -> Article: """Update an existing article instance.""" @@ -152,39 +152,11 @@ def update(self, instance: Article, validated_data: dict) -> Article: try: instance = super().update(instance, validated_data) - - if authors: - instance.authors.set(authors) - if categories: - instance.categories.set(categories) - - # Update related articles if provided - if related_article_ids is not None: - # Clear existing relations - RelatedArticle.objects.filter(from_article=instance).delete() - # Create new relations - for related_article in related_article_ids: - RelatedArticle.objects.create( - from_article=instance, to_article=related_article - ) - + self._handle_relations(instance, authors, categories, related_article_ids) return instance - except DjangoValidationError as e: - # Handle validation errors specifically - raise serializers.ValidationError(e.message_dict) except Exception as e: - # Log the detailed error for debugging - logger.error( - "Error updating article", - extra={ - "instance_id": instance.id if instance else None, - "validated_data": validated_data, - "error": str(e) - }, - exc_info=True - ) - # Return a generic error message to the client - raise serializers.ValidationError({ - "non_field_errors": ["Unable to update article. Please try again later."] + self._handle_error(e, "updating", { + "instance_id": instance.id if instance else None, + "validated_data": validated_data }) \ No newline at end of file