From b458506a78d6a43c3d9cfced8c19f4707fa5492c Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Sun, 24 Nov 2024 20:07:48 +0100 Subject: [PATCH 01/23] feat: auto-redirect url when slug changes. This also keep records of slug histories --- server/apps/research/admin/article_admin.py | 59 ++++++++++++++++++- .../migrations/0012_articleslughistory.py | 28 +++++++++ server/apps/research/models.py | 4 +- server/apps/research/models/__init__.py | 2 +- server/apps/research/models/article.py | 40 +++++++++++++ server/apps/research/views.py | 45 +++++++++----- 6 files changed, 158 insertions(+), 20 deletions(-) create mode 100644 server/apps/research/migrations/0012_articleslughistory.py diff --git a/server/apps/research/admin/article_admin.py b/server/apps/research/admin/article_admin.py index 97e36fd..209c8a4 100644 --- a/server/apps/research/admin/article_admin.py +++ b/server/apps/research/admin/article_admin.py @@ -1,8 +1,11 @@ from django.contrib import admin from django import forms -from apps.research.models import Article, Author +from apps.research.models import Article, Author, ArticleSlugHistory from tinymce.widgets import TinyMCE +from django.utils.html import format_html + + class ArticleForm(forms.ModelForm): class Meta: model = Article @@ -16,16 +19,47 @@ def __init__(self, *args, **kwargs): class ArticleAdmin(admin.ModelAdmin): """Admin interface for the Article model.""" form = ArticleForm + def current_slug_history(self, obj): + """Display the history of URL changes for the article.""" + histories = obj.slug_history.all().order_by('-created_at') + if not histories: + return "No slug changes recorded" + + html = ['
'] + html.append('') + html.append('') + html.append('') + html.append('') + html.append('') + + for history in histories: + html.append('') + html.append(f'') + html.append(f'') + html.append('') + + html.append('
Old SlugChanged At
{history.old_slug}{history.created_at}
') + html.append('
') + + return format_html(''.join(html)) + current_slug_history.short_description = 'Slug Change History' + fieldsets = [ ('Article Details', {'fields': ['title', 'slug', 'authors', 'acknowledgement', 'categories', 'thumb', 'content', 'summary', 'status', 'scheduled_publish_time']}), ('Sponsorship Details', {'fields': ['is_sponsored', 'sponsor_color', 'sponsor_text_color']}), + ('URL Management', { + 'fields': ('current_slug_history',), + 'classes': ('collapse',), + 'description': 'History of URL changes for this article' + }), ] list_display = ('title', 'display_authors', 'status', 'views', 'display_categories', 'min_read', 'created_at', 'scheduled_publish_time') search_fields = ('title', 'authors__user__username', 'authors__twitter_username', 'content') list_per_page = 25 list_filter = ('authors', 'status', 'categories', 'created_at', 'is_sponsored') - readonly_fields = ('views',) + readonly_fields = ('views','current_slug_history',) list_editable = ('status',) + def display_authors(self, obj): """Return a comma-separated list of authors for the article.""" @@ -59,5 +93,26 @@ def has_delete_permission(self, request, obj=None): if obj is not None and not obj.authors.filter(user=request.user).exists(): return False return True +@admin.register(ArticleSlugHistory) +class ArticleSlugHistoryAdmin(admin.ModelAdmin): + """Admin interface for the ArticleSlugHistory model.""" + list_display = ('article_title', 'old_slug', 'current_slug', 'created_at') + list_filter = ('created_at', 'article__title') + search_fields = ('old_slug', 'article__title') + readonly_fields = ('article', 'old_slug', 'created_at') + + def article_title(self, obj): + return obj.article.title + article_title.short_description = 'Article' + + def current_slug(self, obj): + return obj.article.slug + current_slug.short_description = 'Current Slug' + def has_add_permission(self, request): + return False # Prevent manual addition + + def has_delete_permission(self, request, obj=None): + return False # Prevent deletion + admin.site.register(Article, ArticleAdmin) \ No newline at end of file diff --git a/server/apps/research/migrations/0012_articleslughistory.py b/server/apps/research/migrations/0012_articleslughistory.py new file mode 100644 index 0000000..2a064e8 --- /dev/null +++ b/server/apps/research/migrations/0012_articleslughistory.py @@ -0,0 +1,28 @@ +# Generated by Django 5.0.8 on 2024-11-24 18:13 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('research', '0011_remove_article_sponsor_padding'), + ] + + operations = [ + migrations.CreateModel( + name='ArticleSlugHistory', + fields=[ + ('id', models.AutoField(primary_key=True, serialize=False)), + ('old_slug', models.SlugField(max_length=255)), + ('created_at', models.DateTimeField(auto_now_add=True)), + ('article', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='slug_history', to='research.article')), + ], + options={ + 'db_table': 'research_articleslughistory', + 'indexes': [models.Index(fields=['old_slug'], name='research_ar_old_slu_12bba6_idx')], + 'unique_together': {('article', 'old_slug')}, + }, + ), + ] diff --git a/server/apps/research/models.py b/server/apps/research/models.py index c4e3730..1f9a46f 100644 --- a/server/apps/research/models.py +++ b/server/apps/research/models.py @@ -2,6 +2,6 @@ from .category import Category from .author import Author -from .article import Article +from .article import Article, ArticleSlugHistory -__all__ = ['Category', 'Author', 'Article'] +__all__ = ['Category', 'Author', 'Article', 'ArticleSlugHistory'] diff --git a/server/apps/research/models/__init__.py b/server/apps/research/models/__init__.py index aefa2a1..3737c10 100644 --- a/server/apps/research/models/__init__.py +++ b/server/apps/research/models/__init__.py @@ -1,3 +1,3 @@ from .category import Category from .author import Author -from .article import Article \ No newline at end of file +from .article import Article, ArticleSlugHistory \ No newline at end of file diff --git a/server/apps/research/models/article.py b/server/apps/research/models/article.py index 397fc9e..6eaa923 100644 --- a/server/apps/research/models/article.py +++ b/server/apps/research/models/article.py @@ -80,6 +80,29 @@ def build_table_of_contents(self): self.content = str(soup) def save(self, *args, **kwargs): + + """Override the save method to track slug changes.""" + if self.pk: # If this is an existing article + try: + old_instance = Article.objects.get(pk=self.pk) + # Generate new slug first + if not self.slug or self.title_update(): + self.slug = self.generate_unique_slug() + + # Then check if we need to create slug history + if old_instance.slug and old_instance.slug != self.slug: + # Only create history if the slug actually changed and isn't empty + ArticleSlugHistory.objects.create( + article=self, + old_slug=old_instance.slug + ) + except Article.DoesNotExist: + pass + else: + # New article being created + if not self.slug: + self.slug = self.generate_unique_slug() + """Override the save method to generate a unique slug and build table of contents.""" if not self.slug or self.title_update(): self.slug = self.generate_unique_slug() @@ -110,3 +133,20 @@ def title_update(self): if original: return original.title != self.title return False + +class ArticleSlugHistory(models.Model): + """Model to track historical slugs for articles.""" + id = models.AutoField(primary_key=True) + article = models.ForeignKey('Article', on_delete=models.CASCADE, related_name='slug_history') + old_slug = models.SlugField(max_length=255, db_index=True) + created_at = models.DateTimeField(auto_now_add=True) + + class Meta: + unique_together = ('article', 'old_slug') + indexes = [ + models.Index(fields=['old_slug']), + ] + db_table = 'research_articleslughistory' # explicitly set table name + + def __str__(self): + return f"{self.old_slug} -> {self.article.slug}" \ No newline at end of file diff --git a/server/apps/research/views.py b/server/apps/research/views.py index 382b42c..d2c2cfd 100644 --- a/server/apps/research/views.py +++ b/server/apps/research/views.py @@ -1,12 +1,12 @@ from rest_framework.decorators import action from django.db.models import F -from django.shortcuts import render +from django.shortcuts import render, get_object_or_404 from rest_framework import viewsets, status from rest_framework.response import Response import uuid import logging -from .models import Article +from .models import Article, ArticleSlugHistory from .permissions import ArticleUserWritePermission from .serializers import ArticleSerializer, ArticleCreateUpdateSerializer, ArticleListSerializer @@ -59,23 +59,38 @@ def update(self, request, *args, **kwargs): # Custom action to retrieve articles by slug or UUID @action(detail=False, methods=['get'], url_path=r'(?P[-\w0-9a-fA-F]+)') def retrieve_by_identifier(self, request, identifier=None): - """Retrieve an article by slug or UUID.""" + """Retrieve an article by slug or UUID, handling old slugs.""" try: - if self.is_valid_uuid(identifier): + if self.is_valid_uuid(identifier): instance = Article.objects.get(pk=identifier) - else: - instance = Article.objects.get(slug=identifier) - except Article.DoesNotExist: - return Response({'error': 'Article does not exist'}, status=status.HTTP_404_NOT_FOUND) + else: + try: + instance = Article.objects.get(slug=identifier) + except Article.DoesNotExist: + # Check if this is an old slug + slug_history = get_object_or_404(ArticleSlugHistory, old_slug=identifier) + instance = slug_history.article + # Return a redirect response with the new URL + new_url = request.build_absolute_uri().replace( + f'/api/articles/{identifier}/', + f'/api/articles/{instance.slug}/' + ) + return Response({ + 'type': 'redirect', + 'new_url': new_url, + 'data': self.get_serializer(instance).data + }, status=status.HTTP_301_MOVED_PERMANENTLY) + + instance.views = F('views') + 1 + instance.save(update_fields=['views']) + instance.refresh_from_db(fields=['views']) + serializer = self.get_serializer(instance) + return Response({'success': True, 'data': serializer.data}) + except Exception as e: logger.error(f"Error retrieving article by identifier: {e}") - return Response({'error': 'An unexpected error occurred'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) - - instance.views = F('views') + 1 - instance.save(update_fields=['views']) - instance.refresh_from_db(fields=['views']) - serializer = self.get_serializer(instance) - return Response({'success': True, 'data': serializer.data}) + return Response({'error': 'An unexpected error occurred'}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR) # Custom action to retrieve articles by category @action(detail=False, methods=['get'], url_path=r'category/(?P[-\w]+)') From 54015472441b15c1964a6d69f1fdbe221c0f4bba Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Wed, 27 Nov 2024 18:05:17 +0100 Subject: [PATCH 02/23] refactor(url_redirect): updated url_redirect implementation codebase for readability and maintainability --- server/apps/research/admin/article_admin.py | 27 ++---------------- server/apps/research/admin/slug_history.py | 31 +++++++++++++++++++++ server/apps/research/views.py | 8 +++--- 3 files changed, 38 insertions(+), 28 deletions(-) create mode 100644 server/apps/research/admin/slug_history.py diff --git a/server/apps/research/admin/article_admin.py b/server/apps/research/admin/article_admin.py index 209c8a4..352ae59 100644 --- a/server/apps/research/admin/article_admin.py +++ b/server/apps/research/admin/article_admin.py @@ -1,8 +1,8 @@ from django.contrib import admin from django import forms -from apps.research.models import Article, Author, ArticleSlugHistory +from apps.research.models import Article, ArticleSlugHistory from tinymce.widgets import TinyMCE -from django.utils.html import format_html +from .slug_history import current_slug_history @@ -20,28 +20,7 @@ class ArticleAdmin(admin.ModelAdmin): """Admin interface for the Article model.""" form = ArticleForm def current_slug_history(self, obj): - """Display the history of URL changes for the article.""" - histories = obj.slug_history.all().order_by('-created_at') - if not histories: - return "No slug changes recorded" - - html = ['
'] - html.append('') - html.append('') - html.append('') - html.append('') - html.append('') - - for history in histories: - html.append('') - html.append(f'') - html.append(f'') - html.append('') - - html.append('
Old SlugChanged At
{history.old_slug}{history.created_at}
') - html.append('
') - - return format_html(''.join(html)) + return current_slug_history(obj) current_slug_history.short_description = 'Slug Change History' fieldsets = [ diff --git a/server/apps/research/admin/slug_history.py b/server/apps/research/admin/slug_history.py new file mode 100644 index 0000000..fc985d4 --- /dev/null +++ b/server/apps/research/admin/slug_history.py @@ -0,0 +1,31 @@ +from django.utils.html import format_html + +def get_slug_history_table(histories): + """Return the HTML table for the slug history.""" + html = [] + html.append('') + html.append('') + html.append('') + html.append('') + html.append('') + for history in histories: + html.append('') + html.append(f'') + html.append(f'') + html.append('') + html.append('
Old SlugChanged At
{history.old_slug}{history.created_at}
') + return ''.join(html) + +def get_slug_history_html(obj): + """Return the HTML for the slug history.""" + histories = obj.slug_history.all().order_by('-created_at') + if not histories: + return "No slug changes recorded" + html = ['
'] + html.append(get_slug_history_table(histories)) + html.append('
') + return format_html(''.join(html)) + +def current_slug_history(obj): + """Display the history of URL changes for the article.""" + return get_slug_history_html(obj) \ No newline at end of file diff --git a/server/apps/research/views.py b/server/apps/research/views.py index d2c2cfd..56109c5 100644 --- a/server/apps/research/views.py +++ b/server/apps/research/views.py @@ -42,7 +42,7 @@ def create(self, request, *args, **kwargs): return Response(serializer.data, status=status.HTTP_201_CREATED) except Exception as e: logger.error(f"Unexpected error during article creation: {e}") - return Response({'error': 'An unexpected error occurred'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) + return Response({'error': 'Failed to create a new Article'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) def update(self, request, *args, **kwargs): """Handle article update.""" @@ -54,7 +54,7 @@ def update(self, request, *args, **kwargs): return Response(serializer.data, status=status.HTTP_200_OK) except Exception as e: logger.error(f"Unexpected error during article update: {e}") - return Response({'error': 'An unexpected error occurred'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) + return Response({'error': 'Error updating article'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) # Custom action to retrieve articles by slug or UUID @action(detail=False, methods=['get'], url_path=r'(?P[-\w0-9a-fA-F]+)') @@ -89,7 +89,7 @@ def retrieve_by_identifier(self, request, identifier=None): except Exception as e: logger.error(f"Error retrieving article by identifier: {e}") - return Response({'error': 'An unexpected error occurred'}, + return Response({'error': 'Article does not exist'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) # Custom action to retrieve articles by category @@ -102,7 +102,7 @@ def retrieve_by_category(self, request, category=None): return Response({'success': True, 'data': serializer.data}) except Exception as e: logger.error(f"Error retrieving articles by category: {e}") - return Response({'error': 'An unexpected error occurred'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) + return Response({'error': 'Category does not exist'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) def is_valid_uuid(self, value): """Check if the value is a valid UUID.""" From 51ef3eddc48da2006592cb62c70f8b4d0703486c Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Wed, 27 Nov 2024 18:15:16 +0100 Subject: [PATCH 03/23] refactor: changed "Article does not exist" status code to 404 --- server/apps/research/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/apps/research/views.py b/server/apps/research/views.py index 56109c5..a021093 100644 --- a/server/apps/research/views.py +++ b/server/apps/research/views.py @@ -90,7 +90,7 @@ def retrieve_by_identifier(self, request, identifier=None): except Exception as e: logger.error(f"Error retrieving article by identifier: {e}") return Response({'error': 'Article does not exist'}, - status=status.HTTP_500_INTERNAL_SERVER_ERROR) + status=status.HTTP_404_NOT_FOUND) # Custom action to retrieve articles by category @action(detail=False, methods=['get'], url_path=r'category/(?P[-\w]+)') From 690bf4b5b63500d96791f7b636afd9d9de57293e Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 13:19:28 +0100 Subject: [PATCH 04/23] refactor: Used F() expressions within a transaction to prevent race conditions --- server/apps/research/views.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/server/apps/research/views.py b/server/apps/research/views.py index a021093..3e4f174 100644 --- a/server/apps/research/views.py +++ b/server/apps/research/views.py @@ -5,6 +5,7 @@ from rest_framework.response import Response import uuid import logging +from django.db import transaction from .models import Article, ArticleSlugHistory from .permissions import ArticleUserWritePermission @@ -81,9 +82,10 @@ def retrieve_by_identifier(self, request, identifier=None): 'data': self.get_serializer(instance).data }, status=status.HTTP_301_MOVED_PERMANENTLY) - instance.views = F('views') + 1 - instance.save(update_fields=['views']) - instance.refresh_from_db(fields=['views']) + with transaction.atomic(): + instance.views = F('views') + 1 + instance.save(update_fields=['views']) + instance.refresh_from_db(fields=['views']) serializer = self.get_serializer(instance) return Response({'success': True, 'data': serializer.data}) From 4962924510f6873c2a1c9d7b2d86cea928ed445e Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 13:22:28 +0100 Subject: [PATCH 05/23] refactor: fixed security vulnerabilities - html-escaped the old_slug value - moved inline styles to style.css - enhanced table accessibility with ARIA attributes --- server/apps/research/admin/slug_history.py | 20 ++++++++++-------- server/static/style.css | 24 ++++++++++++++++++++++ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/server/apps/research/admin/slug_history.py b/server/apps/research/admin/slug_history.py index fc985d4..ce46dd7 100644 --- a/server/apps/research/admin/slug_history.py +++ b/server/apps/research/admin/slug_history.py @@ -1,20 +1,24 @@ -from django.utils.html import format_html +from django.utils.html import format_html, escape +from django.utils.safestring import mark_safe def get_slug_history_table(histories): """Return the HTML table for the slug history.""" html = [] - html.append('') - html.append('') - html.append('') - html.append('') + html.append('
Old SlugChanged At
') + html.append('') + html.append('') + html.append('') + html.append('') html.append('') + html.append('') for history in histories: html.append('') - html.append(f'') - html.append(f'') + html.append(f'') + html.append(f'') html.append('') + html.append('') html.append('
Slug History Table
Old SlugChanged At
{history.old_slug}{history.created_at}{escape(history.old_slug)}{history.created_at}
') - return ''.join(html) + return mark_safe(''.join(html)) def get_slug_history_html(obj): """Return the HTML for the slug history.""" diff --git a/server/static/style.css b/server/static/style.css index 41b7c09..7421c5d 100644 --- a/server/static/style.css +++ b/server/static/style.css @@ -93,3 +93,27 @@ body { background-color: var(--gray-8); } } + +.slug-history-table { + width: 100%; + border-collapse: collapse; +} + +.slug-history-table th, .slug-history-table td { + padding: 8px; + border: 1px solid #ddd; +} + +.slug-history-table th { + background-color: #f5f5f5; +} + +.slug-history-table caption { + clip: rect(0 0 0 0); + clip-path: inset(50%); + height: 1px; + overflow: hidden; + position: absolute; + white-space: nowrap; + width: 1px; +} \ No newline at end of file From 51b175d82ef9214e64b4b7b2fd01f631330a2b75 Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 13:23:58 +0100 Subject: [PATCH 06/23] chore(fix): handled i18n internationalization for language chooser --- server/core/urls.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/server/core/urls.py b/server/core/urls.py index c9eb82b..92108a4 100644 --- a/server/core/urls.py +++ b/server/core/urls.py @@ -3,6 +3,8 @@ from django.contrib import admin from django.urls import path, include from .token import csrf_token_view +from django.conf.urls.i18n import i18n_patterns + urlpatterns = [ path('admin/', admin.site.urls), @@ -17,5 +19,9 @@ path('tinymce/', include('tinymce.urls')), ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) +urlpatterns += i18n_patterns( + path('i18n/', include('django.conf.urls.i18n')), +) + if settings.DEBUG: urlpatterns += static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) From daccbefb02ace705ea8adc14733b6a5234c76778 Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 14:11:40 +0100 Subject: [PATCH 07/23] Update server/static/style.css --- server/static/style.css | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/server/static/style.css b/server/static/style.css index 7421c5d..141be7a 100644 --- a/server/static/style.css +++ b/server/static/style.css @@ -106,6 +106,16 @@ body { .slug-history-table th { background-color: #f5f5f5; + background-color: var(--gray-1); +} + +@media (prefers-color-scheme: dark) { + .slug-history-table th { + background-color: var(--gray-7); + } + .slug-history-table th, .slug-history-table td { + border-color: var(--gray-6); + } } .slug-history-table caption { From f8090f4622282dd5e1107f47b04efc6d4cbad93b Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 14:32:35 +0100 Subject: [PATCH 08/23] removed i18n config --- server/core/config/base.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/server/core/config/base.py b/server/core/config/base.py index 297b6ea..af40948 100644 --- a/server/core/config/base.py +++ b/server/core/config/base.py @@ -14,6 +14,10 @@ from pathlib import Path from dotenv import load_dotenv +# third party imports +from .celery_config import * +from .mail import * + load_dotenv() from decouple import config @@ -154,6 +158,7 @@ USE_I18N = True + USE_TZ = True # Static files (CSS, JavaScript, Images) @@ -191,7 +196,4 @@ SILENCED_SYSTEM_CHECKS = ["security.W019"] # Tinymce API Config -TINYMCE_API_KEY = os.getenv('TINYMCE_API_KEY') - -from .celery_config import * -from .mail import * \ No newline at end of file +TINYMCE_API_KEY = os.getenv('TINYMCE_API_KEY') \ No newline at end of file From 8813648fb5a78ecaba0d651dde7ec0930573491a Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 14:32:53 +0100 Subject: [PATCH 09/23] removed i18n config --- server/core/urls.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/server/core/urls.py b/server/core/urls.py index 92108a4..e1bbc1d 100644 --- a/server/core/urls.py +++ b/server/core/urls.py @@ -3,7 +3,6 @@ from django.contrib import admin from django.urls import path, include from .token import csrf_token_view -from django.conf.urls.i18n import i18n_patterns urlpatterns = [ @@ -19,9 +18,6 @@ path('tinymce/', include('tinymce.urls')), ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) -urlpatterns += i18n_patterns( - path('i18n/', include('django.conf.urls.i18n')), -) if settings.DEBUG: urlpatterns += static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) From 8fda06e2b152a8c1633732f01eae199135b7b958 Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 14:33:26 +0100 Subject: [PATCH 10/23] update article create/update status code --- server/apps/research/views.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/apps/research/views.py b/server/apps/research/views.py index 3e4f174..1667139 100644 --- a/server/apps/research/views.py +++ b/server/apps/research/views.py @@ -43,6 +43,8 @@ def create(self, request, *args, **kwargs): return Response(serializer.data, status=status.HTTP_201_CREATED) except Exception as e: logger.error(f"Unexpected error during article creation: {e}") + if isinstance(e, serializers.ValidationError): + return Response({'error': str(e)}, status=status.HTTP_400_BAD_REQUEST) return Response({'error': 'Failed to create a new Article'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) def update(self, request, *args, **kwargs): @@ -53,8 +55,10 @@ def update(self, request, *args, **kwargs): serializer.is_valid(raise_exception=True) self.perform_update(serializer) return Response(serializer.data, status=status.HTTP_200_OK) - except Exception as e: + except Exception as e: logger.error(f"Unexpected error during article update: {e}") + if isinstance(e, serializers.ValidationError): + return Response({'error': str(e)}, status=status.HTTP_400_BAD_REQUEST) return Response({'error': 'Error updating article'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) # Custom action to retrieve articles by slug or UUID From ae94f0895eaf151cc8dfa7775641831c2c0ea664 Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 14:43:09 +0100 Subject: [PATCH 11/23] fix: Resolved Information exposure through an exception --- server/apps/research/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/apps/research/views.py b/server/apps/research/views.py index 1667139..0212390 100644 --- a/server/apps/research/views.py +++ b/server/apps/research/views.py @@ -44,7 +44,7 @@ def create(self, request, *args, **kwargs): except Exception as e: logger.error(f"Unexpected error during article creation: {e}") if isinstance(e, serializers.ValidationError): - return Response({'error': str(e)}, status=status.HTTP_400_BAD_REQUEST) + return Response({'error': 'Invalid data provided'}, status=status.HTTP_400_BAD_REQUEST) return Response({'error': 'Failed to create a new Article'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) def update(self, request, *args, **kwargs): @@ -58,7 +58,7 @@ def update(self, request, *args, **kwargs): except Exception as e: logger.error(f"Unexpected error during article update: {e}") if isinstance(e, serializers.ValidationError): - return Response({'error': str(e)}, status=status.HTTP_400_BAD_REQUEST) + return Response({'error': 'Invalid data provided'}, status=status.HTTP_400_BAD_REQUEST) return Response({'error': 'Error updating article'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) # Custom action to retrieve articles by slug or UUID From c066ef2c620941066730769aa00ffaf83d2ea805 Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 14:47:46 +0100 Subject: [PATCH 12/23] Update base.py --- server/core/config/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/config/base.py b/server/core/config/base.py index af40948..7422ce7 100644 --- a/server/core/config/base.py +++ b/server/core/config/base.py @@ -196,4 +196,4 @@ SILENCED_SYSTEM_CHECKS = ["security.W019"] # Tinymce API Config -TINYMCE_API_KEY = os.getenv('TINYMCE_API_KEY') \ No newline at end of file +TINYMCE_API_KEY = config('TINYMCE_API_KEY') \ No newline at end of file From 36284bae37128b2603983f4c5b2ff71b5d7fd62f Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 14:51:14 +0100 Subject: [PATCH 13/23] Updated HTTP status code for non-existent category --- server/apps/research/views.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/apps/research/views.py b/server/apps/research/views.py index 0212390..e131a0b 100644 --- a/server/apps/research/views.py +++ b/server/apps/research/views.py @@ -6,6 +6,8 @@ import uuid import logging from django.db import transaction +from rest_framework import serializers + from .models import Article, ArticleSlugHistory from .permissions import ArticleUserWritePermission @@ -108,7 +110,7 @@ def retrieve_by_category(self, request, category=None): return Response({'success': True, 'data': serializer.data}) except Exception as e: logger.error(f"Error retrieving articles by category: {e}") - return Response({'error': 'Category does not exist'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) + return Response({'error': 'Category does not exist'}, status=status.HTTP_404_NOT_FOUND) def is_valid_uuid(self, value): """Check if the value is a valid UUID.""" From 327f6ce1ea661ae8656897f70b95259c91d50a44 Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 15:07:12 +0100 Subject: [PATCH 14/23] resolved wildcard imports for maintainability --- server/core/config/base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/core/config/base.py b/server/core/config/base.py index 7422ce7..ae1ed98 100644 --- a/server/core/config/base.py +++ b/server/core/config/base.py @@ -15,8 +15,8 @@ from dotenv import load_dotenv # third party imports -from .celery_config import * -from .mail import * +from .celery_config import (CELERY_BROKER_URL, CELERY_RESULT_BACKEND, CELERY_ACCEPT_CONTENT, CELERY_TASK_SERIALIZER, CELERY_RESULT_SERIALIZER, CELERY_TIMEZONE) +from .mail import (SITE_URL, EMAIL_HOST, EMAIL_HOST_USER, EMAIL_HOST_PASSWORD, DEFAULT_FROM_EMAIL, EMAIL_PORT, EMAIL_USE_TLS, EMAIL_USE_SSL, EMAIL_BACKEND) load_dotenv() from decouple import config From fc955995b0dde64b1bf5442ee63b039297c297be Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 16:16:55 +0100 Subject: [PATCH 15/23] Refactor Article model's save method for improved slug handling and status updates --- server/apps/research/models/article.py | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/server/apps/research/models/article.py b/server/apps/research/models/article.py index 6eaa923..c9bf192 100644 --- a/server/apps/research/models/article.py +++ b/server/apps/research/models/article.py @@ -80,14 +80,17 @@ def build_table_of_contents(self): self.content = str(soup) def save(self, *args, **kwargs): + """ + Override the save method to generate a unique slug, build table of contents, + track slug changes, and update article status based on scheduled publish time. + """ + if not self.slug or self.title_update(): + self.slug = self.generate_unique_slug() """Override the save method to track slug changes.""" if self.pk: # If this is an existing article try: - old_instance = Article.objects.get(pk=self.pk) - # Generate new slug first - if not self.slug or self.title_update(): - self.slug = self.generate_unique_slug() + old_instance = Article.objects.get(pk=self.pk) # Then check if we need to create slug history if old_instance.slug and old_instance.slug != self.slug: @@ -97,15 +100,7 @@ def save(self, *args, **kwargs): old_slug=old_instance.slug ) except Article.DoesNotExist: - pass - else: - # New article being created - if not self.slug: - self.slug = self.generate_unique_slug() - - """Override the save method to generate a unique slug and build table of contents.""" - if not self.slug or self.title_update(): - self.slug = self.generate_unique_slug() + pass if self.content: self.build_table_of_contents() From 8f5f4d14a0897c154cf85e5c4782ba1dcf5c21db Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 16:17:24 +0100 Subject: [PATCH 16/23] update http status code message --- server/apps/research/views.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/server/apps/research/views.py b/server/apps/research/views.py index e131a0b..6823199 100644 --- a/server/apps/research/views.py +++ b/server/apps/research/views.py @@ -6,7 +6,6 @@ import uuid import logging from django.db import transaction -from rest_framework import serializers from .models import Article, ArticleSlugHistory @@ -45,7 +44,7 @@ def create(self, request, *args, **kwargs): return Response(serializer.data, status=status.HTTP_201_CREATED) except Exception as e: logger.error(f"Unexpected error during article creation: {e}") - if isinstance(e, serializers.ValidationError): + if isinstance(e, serializer.ValidationError): return Response({'error': 'Invalid data provided'}, status=status.HTTP_400_BAD_REQUEST) return Response({'error': 'Failed to create a new Article'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) @@ -59,7 +58,7 @@ def update(self, request, *args, **kwargs): return Response(serializer.data, status=status.HTTP_200_OK) except Exception as e: logger.error(f"Unexpected error during article update: {e}") - if isinstance(e, serializers.ValidationError): + if isinstance(e, serializer.ValidationError): return Response({'error': 'Invalid data provided'}, status=status.HTTP_400_BAD_REQUEST) return Response({'error': 'Error updating article'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) From 1fa39f320955debb71a1b5824e609559804eb96e Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 17:57:11 +0100 Subject: [PATCH 17/23] update http status code message --- server/apps/research/views.py | 50 +++++++++++++++++------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/server/apps/research/views.py b/server/apps/research/views.py index 6823199..97ca614 100644 --- a/server/apps/research/views.py +++ b/server/apps/research/views.py @@ -67,32 +67,32 @@ def update(self, request, *args, **kwargs): def retrieve_by_identifier(self, request, identifier=None): """Retrieve an article by slug or UUID, handling old slugs.""" try: - if self.is_valid_uuid(identifier): - instance = Article.objects.get(pk=identifier) - else: - try: - instance = Article.objects.get(slug=identifier) - except Article.DoesNotExist: - # Check if this is an old slug - slug_history = get_object_or_404(ArticleSlugHistory, old_slug=identifier) - instance = slug_history.article - # Return a redirect response with the new URL - new_url = request.build_absolute_uri().replace( - f'/api/articles/{identifier}/', - f'/api/articles/{instance.slug}/' - ) - return Response({ - 'type': 'redirect', - 'new_url': new_url, - 'data': self.get_serializer(instance).data - }, status=status.HTTP_301_MOVED_PERMANENTLY) - with transaction.atomic(): - instance.views = F('views') + 1 - instance.save(update_fields=['views']) - instance.refresh_from_db(fields=['views']) - serializer = self.get_serializer(instance) - return Response({'success': True, 'data': serializer.data}) + if self.is_valid_uuid(identifier): + instance = Article.objects.get(pk=identifier) + else: + try: + instance = Article.objects.get(slug=identifier) + except Article.DoesNotExist: + # Check if this is an old slug + slug_history = get_object_or_404(ArticleSlugHistory, old_slug=identifier) + instance = slug_history.article + # Return a redirect response with the new URL + new_url = request.build_absolute_uri().replace( + f'/api/articles/{identifier}/', + f'/api/articles/{instance.slug}/' + ) + return Response({ + 'type': 'redirect', + 'new_url': new_url, + 'data': self.get_serializer(instance).data + }, status=status.HTTP_301_MOVED_PERMANENTLY) + + instance.views = F('views') + 1 + instance.save(update_fields=['views']) + instance.refresh_from_db(fields=['views']) + serializer = self.get_serializer(instance) + return Response({'success': True, 'data': serializer.data}) except Exception as e: logger.error(f"Error retrieving article by identifier: {e}") From 5085a3f5bd4faec31a171349674f85b0682f7013 Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 18:08:07 +0100 Subject: [PATCH 18/23] fix save override for title_update remove "blank=true" for author field --- .../migrations/0013_alter_article_authors.py | 18 ++++++++++++++++++ server/apps/research/models/article.py | 16 ++++++++-------- 2 files changed, 26 insertions(+), 8 deletions(-) create mode 100644 server/apps/research/migrations/0013_alter_article_authors.py diff --git a/server/apps/research/migrations/0013_alter_article_authors.py b/server/apps/research/migrations/0013_alter_article_authors.py new file mode 100644 index 0000000..d9ac19f --- /dev/null +++ b/server/apps/research/migrations/0013_alter_article_authors.py @@ -0,0 +1,18 @@ +# Generated by Django 5.0.8 on 2024-11-29 17:04 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('research', '0012_articleslughistory'), + ] + + operations = [ + migrations.AlterField( + model_name='article', + name='authors', + field=models.ManyToManyField(related_name='articles', to='research.author'), + ), + ] diff --git a/server/apps/research/models/article.py b/server/apps/research/models/article.py index c9bf192..2a0f57b 100644 --- a/server/apps/research/models/article.py +++ b/server/apps/research/models/article.py @@ -26,7 +26,7 @@ class Article(BaseModel): content = HTMLField(blank=True, null=True) summary = models.TextField(blank=True) acknowledgement = HTMLField(blank=True, null=True) - authors = models.ManyToManyField(Author, blank=True, related_name='articles') + authors = models.ManyToManyField(Author, related_name='articles') slug = models.SlugField(max_length=255, blank=True, db_index=True) categories = models.ManyToManyField(Category, blank=True, related_name='articles') thumb = models.ImageField(upload_to='images/', default=get_default_thumb, blank=True) @@ -80,17 +80,17 @@ def build_table_of_contents(self): self.content = str(soup) def save(self, *args, **kwargs): - """ - Override the save method to generate a unique slug, build table of contents, - track slug changes, and update article status based on scheduled publish time. - """ + """Override the save method to generate a unique slug and build table of contents.""" if not self.slug or self.title_update(): self.slug = self.generate_unique_slug() """Override the save method to track slug changes.""" if self.pk: # If this is an existing article try: - old_instance = Article.objects.get(pk=self.pk) + old_instance = Article.objects.get(pk=self.pk) + # Generate new slug first + if not self.slug or self.title_update(): + self.slug = self.generate_unique_slug() # Then check if we need to create slug history if old_instance.slug and old_instance.slug != self.slug: @@ -100,8 +100,8 @@ def save(self, *args, **kwargs): old_slug=old_instance.slug ) except Article.DoesNotExist: - pass - + pass + if self.content: self.build_table_of_contents() From 5742a1e537303ee501020184ddc41d1d0083fc88 Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 18:18:43 +0100 Subject: [PATCH 19/23] disable "PERIODIC TASKS" from admin panel --- server/core/config/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/config/base.py b/server/core/config/base.py index ae1ed98..eb3de97 100644 --- a/server/core/config/base.py +++ b/server/core/config/base.py @@ -57,7 +57,7 @@ THIRD_PARTY_APPS = [ 'corsheaders', - 'django_celery_beat', + #'django_celery_beat', 'tinymce', ] From 2223ec48e5a8eb4f9d227025a3b9f7d12822a1a6 Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 18:32:09 +0100 Subject: [PATCH 20/23] update the exception handling to reference django's rest serializer module --- server/apps/research/views.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/server/apps/research/views.py b/server/apps/research/views.py index 97ca614..adfa9d8 100644 --- a/server/apps/research/views.py +++ b/server/apps/research/views.py @@ -6,6 +6,7 @@ import uuid import logging from django.db import transaction +from rest_framework import serializers from .models import Article, ArticleSlugHistory @@ -44,7 +45,7 @@ def create(self, request, *args, **kwargs): return Response(serializer.data, status=status.HTTP_201_CREATED) except Exception as e: logger.error(f"Unexpected error during article creation: {e}") - if isinstance(e, serializer.ValidationError): + if isinstance(e, serializers.ValidationError): return Response({'error': 'Invalid data provided'}, status=status.HTTP_400_BAD_REQUEST) return Response({'error': 'Failed to create a new Article'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) @@ -58,7 +59,7 @@ def update(self, request, *args, **kwargs): return Response(serializer.data, status=status.HTTP_200_OK) except Exception as e: logger.error(f"Unexpected error during article update: {e}") - if isinstance(e, serializer.ValidationError): + if isinstance(e, serializers.ValidationError): return Response({'error': 'Invalid data provided'}, status=status.HTTP_400_BAD_REQUEST) return Response({'error': 'Error updating article'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) @@ -88,9 +89,9 @@ def retrieve_by_identifier(self, request, identifier=None): 'data': self.get_serializer(instance).data }, status=status.HTTP_301_MOVED_PERMANENTLY) - instance.views = F('views') + 1 - instance.save(update_fields=['views']) - instance.refresh_from_db(fields=['views']) + instance.views = F('views') + 1 + instance.save(update_fields=['views']) + instance.refresh_from_db(fields=['views']) serializer = self.get_serializer(instance) return Response({'success': True, 'data': serializer.data}) From c1058e8d1ad1c6607cad49681b2354aeab6b4bc7 Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Fri, 29 Nov 2024 18:40:27 +0100 Subject: [PATCH 21/23] Sanitize URL components to prevent path traversal --- server/apps/research/views.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/apps/research/views.py b/server/apps/research/views.py index adfa9d8..eb355b9 100644 --- a/server/apps/research/views.py +++ b/server/apps/research/views.py @@ -7,6 +7,7 @@ import logging from django.db import transaction from rest_framework import serializers +from urllib.parse import quote from .models import Article, ArticleSlugHistory @@ -80,8 +81,8 @@ def retrieve_by_identifier(self, request, identifier=None): instance = slug_history.article # Return a redirect response with the new URL new_url = request.build_absolute_uri().replace( - f'/api/articles/{identifier}/', - f'/api/articles/{instance.slug}/' + f'/api/articles/{quote(identifier)}/', + f'/api/articles/{quote(instance.slug)}/' ) return Response({ 'type': 'redirect', From 092f1987a1ea18e2f95f9b1f232b5f542df1b55e Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Mon, 2 Dec 2024 09:22:32 +0100 Subject: [PATCH 22/23] - revert blank=True for authors - Add migration file to revert - uncomment celery from settings.py APPS --- .../migrations/0014_alter_article_authors.py | 18 ++++++++++++++++++ server/apps/research/models/article.py | 2 +- server/core/config/base.py | 2 +- 3 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 server/apps/research/migrations/0014_alter_article_authors.py diff --git a/server/apps/research/migrations/0014_alter_article_authors.py b/server/apps/research/migrations/0014_alter_article_authors.py new file mode 100644 index 0000000..493f07c --- /dev/null +++ b/server/apps/research/migrations/0014_alter_article_authors.py @@ -0,0 +1,18 @@ +# Generated by Django 5.0.8 on 2024-12-02 08:20 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('research', '0013_alter_article_authors'), + ] + + operations = [ + migrations.AlterField( + model_name='article', + name='authors', + field=models.ManyToManyField(blank=True, related_name='articles', to='research.author'), + ), + ] diff --git a/server/apps/research/models/article.py b/server/apps/research/models/article.py index 2a0f57b..6172bf2 100644 --- a/server/apps/research/models/article.py +++ b/server/apps/research/models/article.py @@ -26,7 +26,7 @@ class Article(BaseModel): content = HTMLField(blank=True, null=True) summary = models.TextField(blank=True) acknowledgement = HTMLField(blank=True, null=True) - authors = models.ManyToManyField(Author, related_name='articles') + authors = models.ManyToManyField(Author, blank=True, related_name='articles') slug = models.SlugField(max_length=255, blank=True, db_index=True) categories = models.ManyToManyField(Category, blank=True, related_name='articles') thumb = models.ImageField(upload_to='images/', default=get_default_thumb, blank=True) diff --git a/server/core/config/base.py b/server/core/config/base.py index eb3de97..ae1ed98 100644 --- a/server/core/config/base.py +++ b/server/core/config/base.py @@ -57,7 +57,7 @@ THIRD_PARTY_APPS = [ 'corsheaders', - #'django_celery_beat', + 'django_celery_beat', 'tinymce', ] From b19151e144e1b6766b528750a99a5fca5f7ace8c Mon Sep 17 00:00:00 2001 From: Happy Felix Chukwuma Date: Mon, 2 Dec 2024 09:48:50 +0100 Subject: [PATCH 23/23] atomicize slugHistory creation --- server/apps/research/models/article.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/server/apps/research/models/article.py b/server/apps/research/models/article.py index 6172bf2..72cca9d 100644 --- a/server/apps/research/models/article.py +++ b/server/apps/research/models/article.py @@ -10,6 +10,7 @@ import json from bs4 import BeautifulSoup import uuid +from django.db import transaction def get_default_thumb(): return f"{settings.MEDIA_URL}images/2077-Collective.png" @@ -95,10 +96,11 @@ def save(self, *args, **kwargs): # Then check if we need to create slug history if old_instance.slug and old_instance.slug != self.slug: # Only create history if the slug actually changed and isn't empty - ArticleSlugHistory.objects.create( - article=self, - old_slug=old_instance.slug - ) + with transaction.atomic(): + ArticleSlugHistory.objects.create( + article=self, + old_slug=old_instance.slug + ) except Article.DoesNotExist: pass