Refactor error_response function for improved argument handling
- Enhanced the `error_response` function to support backward compatibility by normalizing arguments when positional arguments are misused. - Updated various views to pass `None` for the `errors` parameter in `error_response` calls, ensuring consistent response formatting. - Adjusted logging in `ContentSyncService` and `WordPressClient` to use debug level for expected 401 errors, improving log clarity. - Removed deprecated fields from serializers and views, streamlining content management processes.
This commit is contained in:
@@ -5,6 +5,8 @@ Provides consistent response format across all endpoints
|
||||
from rest_framework.response import Response
|
||||
from rest_framework import status
|
||||
import uuid
|
||||
from typing import Any
|
||||
from django.http import HttpRequest
|
||||
|
||||
|
||||
def get_request_id(request):
|
||||
@@ -74,6 +76,28 @@ def error_response(error=None, errors=None, status_code=status.HTTP_400_BAD_REQU
|
||||
'success': False,
|
||||
}
|
||||
|
||||
# Backwards compatibility: some callers used positional args in the order
|
||||
# (error, status_code, request) which maps to (error, errors, status_code=request)
|
||||
# causing `status_code` to be a Request object and raising TypeError.
|
||||
# Detect this misuse and normalize arguments:
|
||||
try:
|
||||
if request is None and status_code is not None:
|
||||
# If status_code appears to be a Request object, shift arguments
|
||||
if isinstance(status_code, HttpRequest) or hasattr(status_code, 'META'):
|
||||
# original call looked like: error_response(msg, status.HTTP_400_BAD_REQUEST, request)
|
||||
# which resulted in: errors = status.HTTP_400..., status_code = request
|
||||
request = status_code
|
||||
# If `errors` holds an int-like HTTP status, use it as status_code
|
||||
if isinstance(errors, int):
|
||||
status_code = errors
|
||||
errors = None
|
||||
else:
|
||||
# fallback to default 400
|
||||
status_code = status.HTTP_400_BAD_REQUEST
|
||||
except Exception:
|
||||
# Defensive: if introspection fails, continue with provided args
|
||||
pass
|
||||
|
||||
if error:
|
||||
response_data['error'] = error
|
||||
elif status_code == status.HTTP_400_BAD_REQUEST:
|
||||
|
||||
@@ -409,7 +409,7 @@ class ContentSyncService:
|
||||
)
|
||||
synced_count += len(tag_records)
|
||||
|
||||
# Sync WooCommerce product categories if available
|
||||
# Sync WooCommerce product categories if available (401 is expected if WooCommerce not installed or credentials missing)
|
||||
try:
|
||||
product_categories = client.get_product_categories(per_page=100)
|
||||
product_category_records = [
|
||||
@@ -431,7 +431,8 @@ class ContentSyncService:
|
||||
)
|
||||
synced_count += len(product_category_records)
|
||||
except Exception as e:
|
||||
logger.warning(f"WooCommerce not available or error fetching product categories: {e}")
|
||||
# Silently skip WooCommerce if not available (401 means no consumer key/secret configured or plugin not installed)
|
||||
logger.debug(f"WooCommerce product categories not available: {e}")
|
||||
|
||||
return {
|
||||
'success': True,
|
||||
@@ -588,10 +589,10 @@ class ContentSyncService:
|
||||
'synced_count': synced_count
|
||||
}
|
||||
except Exception as e:
|
||||
logger.error(f"Error syncing products from WordPress: {e}", exc_info=True)
|
||||
# Silently skip products if WooCommerce auth fails (expected if consumer key/secret not configured)
|
||||
logger.debug(f"WooCommerce products not synced: {e}")
|
||||
return {
|
||||
'success': False,
|
||||
'error': str(e),
|
||||
'success': True,
|
||||
'synced_count': 0
|
||||
}
|
||||
|
||||
|
||||
@@ -54,6 +54,7 @@ class IntegrationViewSet(SiteSectorModelViewSet):
|
||||
else:
|
||||
return error_response(
|
||||
result.get('message', 'Connection test failed'),
|
||||
None,
|
||||
status.HTTP_400_BAD_REQUEST,
|
||||
request
|
||||
)
|
||||
@@ -78,14 +79,14 @@ class IntegrationViewSet(SiteSectorModelViewSet):
|
||||
site_url = request.data.get('site_url')
|
||||
|
||||
if not site_id:
|
||||
return error_response('site_id is required', status.HTTP_400_BAD_REQUEST, request)
|
||||
return error_response('site_id is required', None, status.HTTP_400_BAD_REQUEST, request)
|
||||
|
||||
# Verify site exists
|
||||
from igny8_core.auth.models import Site
|
||||
try:
|
||||
site = Site.objects.get(id=int(site_id))
|
||||
except (Site.DoesNotExist, ValueError, TypeError):
|
||||
return error_response('Site not found or invalid', status.HTTP_404_NOT_FOUND, request)
|
||||
return error_response('Site not found or invalid', None, status.HTTP_404_NOT_FOUND, request)
|
||||
|
||||
# Authentication: accept either authenticated user OR matching API key in body
|
||||
api_key = request.data.get('api_key') or api_key
|
||||
@@ -107,7 +108,7 @@ class IntegrationViewSet(SiteSectorModelViewSet):
|
||||
authenticated = True
|
||||
|
||||
if not authenticated:
|
||||
return error_response('Authentication credentials were not provided.', status.HTTP_403_FORBIDDEN, request)
|
||||
return error_response('Authentication credentials were not provided.', None, status.HTTP_403_FORBIDDEN, request)
|
||||
|
||||
# Try to find an existing integration for this site+platform
|
||||
integration = SiteIntegration.objects.filter(site=site, platform='wordpress').first()
|
||||
@@ -128,7 +129,7 @@ class IntegrationViewSet(SiteSectorModelViewSet):
|
||||
if result.get('success'):
|
||||
return success_response(result, request=request)
|
||||
else:
|
||||
return error_response(result.get('message', 'Connection test failed'), status.HTTP_400_BAD_REQUEST, request)
|
||||
return error_response(result.get('message', 'Connection test failed'), None, status.HTTP_400_BAD_REQUEST, request)
|
||||
|
||||
@action(detail=True, methods=['post'])
|
||||
def sync(self, request, pk=None):
|
||||
@@ -275,6 +276,7 @@ class IntegrationViewSet(SiteSectorModelViewSet):
|
||||
except (ValueError, TypeError):
|
||||
return error_response(
|
||||
'Invalid site_id',
|
||||
None,
|
||||
status.HTTP_400_BAD_REQUEST,
|
||||
request
|
||||
)
|
||||
@@ -286,6 +288,7 @@ class IntegrationViewSet(SiteSectorModelViewSet):
|
||||
except Site.DoesNotExist:
|
||||
return error_response(
|
||||
'Site not found',
|
||||
None,
|
||||
status.HTTP_404_NOT_FOUND,
|
||||
request
|
||||
)
|
||||
@@ -314,6 +317,7 @@ class IntegrationViewSet(SiteSectorModelViewSet):
|
||||
except (ValueError, TypeError):
|
||||
return error_response(
|
||||
'Invalid site_id',
|
||||
None,
|
||||
status.HTTP_400_BAD_REQUEST,
|
||||
request
|
||||
)
|
||||
@@ -325,6 +329,7 @@ class IntegrationViewSet(SiteSectorModelViewSet):
|
||||
except Site.DoesNotExist:
|
||||
return error_response(
|
||||
'Site not found',
|
||||
None,
|
||||
status.HTTP_404_NOT_FOUND,
|
||||
request
|
||||
)
|
||||
@@ -342,6 +347,7 @@ class IntegrationViewSet(SiteSectorModelViewSet):
|
||||
if not integrations.exists():
|
||||
return error_response(
|
||||
'No active integrations found for this site',
|
||||
None,
|
||||
status.HTTP_400_BAD_REQUEST,
|
||||
request
|
||||
)
|
||||
@@ -381,6 +387,7 @@ class IntegrationViewSet(SiteSectorModelViewSet):
|
||||
except (ValueError, TypeError):
|
||||
return error_response(
|
||||
'Invalid site_id',
|
||||
None,
|
||||
status.HTTP_400_BAD_REQUEST,
|
||||
request
|
||||
)
|
||||
@@ -392,6 +399,7 @@ class IntegrationViewSet(SiteSectorModelViewSet):
|
||||
except Site.DoesNotExist:
|
||||
return error_response(
|
||||
'Site not found',
|
||||
None,
|
||||
status.HTTP_404_NOT_FOUND,
|
||||
request
|
||||
)
|
||||
@@ -418,6 +426,7 @@ class IntegrationViewSet(SiteSectorModelViewSet):
|
||||
except (ValueError, TypeError):
|
||||
return error_response(
|
||||
'Invalid site_id',
|
||||
None,
|
||||
status.HTTP_400_BAD_REQUEST,
|
||||
request
|
||||
)
|
||||
@@ -429,6 +438,7 @@ class IntegrationViewSet(SiteSectorModelViewSet):
|
||||
except Site.DoesNotExist:
|
||||
return error_response(
|
||||
'Site not found',
|
||||
None,
|
||||
status.HTTP_404_NOT_FOUND,
|
||||
request
|
||||
)
|
||||
|
||||
@@ -185,8 +185,8 @@ class ContentIdeasSerializer(serializers.ModelSerializer):
|
||||
'id',
|
||||
'idea_title',
|
||||
'description',
|
||||
'content_structure',
|
||||
'content_type',
|
||||
'site_entity_type',
|
||||
'cluster_role',
|
||||
'target_keywords',
|
||||
'keyword_cluster_id',
|
||||
'keyword_cluster_name',
|
||||
|
||||
@@ -1018,16 +1018,14 @@ class ContentIdeasViewSet(SiteSectorModelViewSet):
|
||||
keywords=idea.target_keywords or '',
|
||||
cluster=idea.keyword_cluster,
|
||||
idea=idea,
|
||||
content_structure=idea.content_structure,
|
||||
content_type=idea.content_type,
|
||||
status='queued',
|
||||
account=idea.account,
|
||||
site=idea.site,
|
||||
sector=idea.sector,
|
||||
# Stage 3: Inherit entity metadata
|
||||
entity_type=idea.site_entity_type or 'blog_post',
|
||||
# Stage 3: Inherit entity metadata (use standardized fields)
|
||||
entity_type=(idea.site_entity_type or 'post'),
|
||||
taxonomy=idea.taxonomy,
|
||||
cluster_role=idea.cluster_role or 'hub',
|
||||
cluster_role=(idea.cluster_role or 'hub'),
|
||||
)
|
||||
created_tasks.append(task.id)
|
||||
# Update idea status
|
||||
|
||||
@@ -25,8 +25,7 @@ class TasksSerializer(serializers.ModelSerializer):
|
||||
content_html = serializers.SerializerMethodField()
|
||||
content_primary_keyword = serializers.SerializerMethodField()
|
||||
content_secondary_keywords = serializers.SerializerMethodField()
|
||||
content_tags = serializers.SerializerMethodField()
|
||||
content_categories = serializers.SerializerMethodField()
|
||||
# tags/categories removed — use taxonomies M2M on Content
|
||||
|
||||
class Meta:
|
||||
model = Tasks
|
||||
@@ -40,25 +39,16 @@ class TasksSerializer(serializers.ModelSerializer):
|
||||
'sector_name',
|
||||
'idea_id',
|
||||
'idea_title',
|
||||
'content_structure',
|
||||
'content_type',
|
||||
'status',
|
||||
'content',
|
||||
'word_count',
|
||||
'meta_title',
|
||||
'meta_description',
|
||||
# task-level raw content/seo fields removed — stored on Content
|
||||
'content_html',
|
||||
'content_primary_keyword',
|
||||
'content_secondary_keywords',
|
||||
'content_tags',
|
||||
'content_categories',
|
||||
'assigned_post_id',
|
||||
'post_url',
|
||||
'created_at',
|
||||
'updated_at',
|
||||
'site_id',
|
||||
'sector_id',
|
||||
'account_id',
|
||||
'created_at',
|
||||
'updated_at',
|
||||
]
|
||||
read_only_fields = ['id', 'created_at', 'updated_at', 'account_id']
|
||||
|
||||
@@ -120,12 +110,18 @@ class TasksSerializer(serializers.ModelSerializer):
|
||||
return record.secondary_keywords if record else []
|
||||
|
||||
def get_content_tags(self, obj):
|
||||
# tags removed; derive taxonomies from Content.taxonomies if needed
|
||||
record = self._get_content_record(obj)
|
||||
return record.tags if record else []
|
||||
if not record:
|
||||
return []
|
||||
return [t.name for t in record.taxonomies.all()]
|
||||
|
||||
def get_content_categories(self, obj):
|
||||
# categories removed; derive hierarchical taxonomies from Content.taxonomies
|
||||
record = self._get_content_record(obj)
|
||||
return record.categories if record else []
|
||||
if not record:
|
||||
return []
|
||||
return [t.name for t in record.taxonomies.filter(taxonomy_type__in=['category','product_cat'])]
|
||||
|
||||
def _cluster_map_qs(self, obj):
|
||||
return ContentClusterMap.objects.filter(task=obj).select_related('cluster')
|
||||
@@ -269,8 +265,6 @@ class ContentSerializer(serializers.ModelSerializer):
|
||||
'meta_description',
|
||||
'primary_keyword',
|
||||
'secondary_keywords',
|
||||
'tags',
|
||||
'categories',
|
||||
'status',
|
||||
'generated_at',
|
||||
'updated_at',
|
||||
|
||||
@@ -457,7 +457,11 @@ class WordPressClient:
|
||||
}
|
||||
for prod in products
|
||||
]
|
||||
logger.warning(f"Failed to fetch products: HTTP {response.status_code}")
|
||||
# Log as debug if 401 (expected if WooCommerce not configured)
|
||||
if response.status_code == 401:
|
||||
logger.debug(f"WooCommerce products require authentication: HTTP {response.status_code}")
|
||||
else:
|
||||
logger.warning(f"Failed to fetch products: HTTP {response.status_code}")
|
||||
return []
|
||||
except Exception as e:
|
||||
logger.error(f"Error fetching WooCommerce products: {e}")
|
||||
@@ -533,7 +537,11 @@ class WordPressClient:
|
||||
}
|
||||
for cat in categories
|
||||
]
|
||||
logger.warning(f"Failed to fetch product categories: HTTP {response.status_code}")
|
||||
# Log as debug if 401 (expected if WooCommerce not configured)
|
||||
if response.status_code == 401:
|
||||
logger.debug(f"WooCommerce product categories require authentication: HTTP {response.status_code}")
|
||||
else:
|
||||
logger.warning(f"Failed to fetch product categories: HTTP {response.status_code}")
|
||||
return []
|
||||
except Exception as e:
|
||||
logger.error(f"Error fetching WooCommerce product categories: {e}")
|
||||
|
||||
Reference in New Issue
Block a user