Browse Source

Plenty fixes, namespaces for misago links, fix #249

Rafał Pitoń 11 years ago
parent
commit
31abe6aec2

+ 6 - 4
docs/developers/coding_style.rst

@@ -53,15 +53,17 @@ Link names should correspond to views and patterns whenever possible. This means
    There are many approaches to this "problem" but preffered one is to use more descriptive name instead. Ask yourself what your view shows to the user and use this name instead. This means "threads_list" instead of "forum" or "user_profile" instead of "user".
    There are many approaches to this "problem" but preffered one is to use more descriptive name instead. Ask yourself what your view shows to the user and use this name instead. This means "threads_list" instead of "forum" or "user_profile" instead of "user".
 
 
 
 
-Avoid temptation to prefix links with app names. Good link name should point at revelant app without that. If there's name collision, you should rethink your views because there's chance you are either repeating yourself or planned your app structure incorrectly.
-
-
 Link Parameters
 Link Parameters
 ---------------
 ---------------
 
 
 Links pointing at classes instead of functions should use lowercase letters and undersores. This means that link pointing at "ForumThreads" should be named "forum_threads".
 Links pointing at classes instead of functions should use lowercase letters and undersores. This means that link pointing at "ForumThreads" should be named "forum_threads".
 
 
-If link parameters represent model fields, name them using model_field scheme. This means that if your link contains UserWarn's id and slug, name those parameters user_warn_id and user_warn_slug in your link and view. In rare cases you may want link parameters point at two instances of same model. If this is the case add use more descriptive prefix instead of one from model name (ergo "quoted_post").
+If link parameters represent model fields, name them using model_field scheme. This means that if your link contains UserWarn's id and slug, name those parameters userwarn_id and userwarn_slug in your link and view.
+
+.. note::
+   Notice that paramerter for this model is `userwarn_slug`, not `user_warn_slug`. This is important because when model slug validation fails, Misago error handler seeks for lowercase class name in link parameters.
+
+In rare cases you may want link parameters point at two instances of same model. If this is the case add use more descriptive prefix instead of one from model name (ergo "quoted_post").
 
 
 
 
 Views and Forms
 Views and Forms

+ 1 - 1
misago/core/cachebuster.py

@@ -58,9 +58,9 @@ class CacheBusterController(object):
 
 
     def invalidate_cache(self, cache):
     def invalidate_cache(self, cache):
         from misago.core.models import CacheVersion
         from misago.core.models import CacheVersion
+        self.cache[cache] += 1
         CacheVersion.objects.filter(cache=cache).update(
         CacheVersion.objects.filter(cache=cache).update(
             version=models.F('version') + 1)
             version=models.F('version') + 1)
-        self.cache[cache] += 1
         default_cache.delete(CACHE_KEY)
         default_cache.delete(CACHE_KEY)
 
 
     def invalidate_all(self):
     def invalidate_all(self):

+ 19 - 11
misago/core/exceptionhandler.py

@@ -1,5 +1,6 @@
 from django.core.exceptions import PermissionDenied
 from django.core.exceptions import PermissionDenied
-from django.http import Http404
+from django.core.urlresolvers import reverse
+from django.http import Http404, HttpResponsePermanentRedirect
 from misago.core import errorpages
 from misago.core import errorpages
 from misago.core.exceptions import OutdatedSlug
 from misago.core.exceptions import OutdatedSlug
 
 
@@ -11,24 +12,31 @@ def is_misago_exception(exception):
     return exception.__class__ in HANDLED_EXCEPTIONS
     return exception.__class__ in HANDLED_EXCEPTIONS
 
 
 
 
-def _get_exception_message(exception):
-    try:
-        return exception.args[0]
-    except IndexError:
-        return None
-
-
 def handle_http404_exception(request, exception):
 def handle_http404_exception(request, exception):
     return errorpages.page_not_found(request)
     return errorpages.page_not_found(request)
 
 
 
 
 def handle_outdated_slug_exception(request, exception):
 def handle_outdated_slug_exception(request, exception):
-    raise NotImplementedError()
+    matched_url = request.resolver_match.url_name
+    if request.resolver_match.namespace:
+        matched_url = '%s:%s' % (request.resolver_match, matched_url)
+
+    model = exception.args[0]
+    model_name = model.__class__.__name__.lower()
+    url_kwargs = request.resolver_match.kwargs
+    url_kwargs['%s_slug' % model_name] = model.slug
+
+    new_url = reverse(matched_url, kwargs=url_kwargs)
+    return HttpResponsePermanentRedirect(new_url)
 
 
 
 
 def handle_permission_denied_exception(request, exception):
 def handle_permission_denied_exception(request, exception):
-    return errorpages.permission_denied(request,
-                                        _get_exception_message(exception))
+    try:
+        error_message = exception.args[0]
+    except IndexError:
+        error_message = None
+
+    return errorpages.permission_denied(request, error_message)
 
 
 
 
 EXCEPTION_HANDLERS = (
 EXCEPTION_HANDLERS = (

+ 1 - 1
misago/core/shortcuts.py

@@ -2,6 +2,6 @@ from django.shortcuts import *
 from misago.core.exceptions import OutdatedSlug
 from misago.core.exceptions import OutdatedSlug
 
 
 
 
-def check_object_slug(model, slug):
+def validate_slug(model, slug):
     if model.slug != slug:
     if model.slug != slug:
         raise OutdatedSlug(model)
         raise OutdatedSlug(model)

+ 4 - 0
misago/core/testproject/models.py

@@ -0,0 +1,4 @@
+class Model(object):
+    def __init__(self, pk, slug):
+        self.id, self.pk = pk, pk
+        self.slug = slug

+ 2 - 1
misago/core/testproject/urls.py

@@ -1,12 +1,13 @@
 from django.conf.urls import patterns, include, url
 from django.conf.urls import patterns, include, url
 
 
 urlpatterns = patterns('',
 urlpatterns = patterns('',
-    url(r'^forum/', include('misago.urls')),
+    url(r'^forum/', include('misago.urls', namespace='misago')),
 )
 )
 
 
 urlpatterns += patterns('misago.core.testproject.views',
 urlpatterns += patterns('misago.core.testproject.views',
     url(r'^forum/test-403/$', 'raise_misago_403', name='raise_misago_403'),
     url(r'^forum/test-403/$', 'raise_misago_403', name='raise_misago_403'),
     url(r'^forum/test-404/$', 'raise_misago_404', name='raise_misago_404'),
     url(r'^forum/test-404/$', 'raise_misago_404', name='raise_misago_404'),
+    url(r'^forum/test-valid-slug/(?P<model_slug>[a-z0-9\-]+)-(?P<model_id>\d+)/$', 'validate_slug_view', name='validate_slug_view'),
     url(r'^test-403/$', 'raise_403', name='raise_403'),
     url(r'^test-403/$', 'raise_403', name='raise_403'),
     url(r'^test-404/$', 'raise_404', name='raise_404'),
     url(r'^test-404/$', 'raise_404', name='raise_404'),
 )
 )

+ 8 - 0
misago/core/testproject/views.py

@@ -1,6 +1,8 @@
 from django.core.exceptions import PermissionDenied
 from django.core.exceptions import PermissionDenied
 from django.http import Http404, HttpResponse
 from django.http import Http404, HttpResponse
 from misago.core import errorpages
 from misago.core import errorpages
+from misago.core.shortcuts import validate_slug
+from misago.core.testproject.models import Model
 
 
 
 
 def raise_misago_403(request):
 def raise_misago_403(request):
@@ -19,6 +21,12 @@ def raise_404(request):
     raise Http404()
     raise Http404()
 
 
 
 
+def validate_slug_view(request, model_id, model_slug):
+    model = Model(int(model_id), 'eric-the-fish')
+    validate_slug(model, model_slug)
+    return HttpResponse("Allright!")
+
+
 @errorpages.shared_403_exception_handler
 @errorpages.shared_403_exception_handler
 def mock_custom_403_error_page(request):
 def mock_custom_403_error_page(request):
     return HttpResponse("Custom 403", status=403)
     return HttpResponse("Custom 403", status=403)

+ 1 - 1
misago/core/tests/test_errorpages.py

@@ -23,7 +23,7 @@ class CustomErrorPagesTests(TestCase):
     urls = 'misago.core.testproject.urlswitherrorhandlers'
     urls = 'misago.core.testproject.urlswitherrorhandlers'
 
 
     def setUp(self):
     def setUp(self):
-        self.misago_request = RequestFactory().get(reverse('forum_index'))
+        self.misago_request = RequestFactory().get(reverse('misago:index'))
         self.site_request = RequestFactory().get(reverse('raise_403'))
         self.site_request = RequestFactory().get(reverse('raise_403'))
 
 
     def test_shared_403_decorator(self):
     def test_shared_403_decorator(self):

+ 0 - 91
misago/core/tests/test_exceptionhandler.py

@@ -1,4 +1,3 @@
-from django.core.urlresolvers import reverse
 from django.http import Http404
 from django.http import Http404
 from django.core import exceptions as django_exceptions
 from django.core import exceptions as django_exceptions
 from django.core.exceptions import PermissionDenied
 from django.core.exceptions import PermissionDenied
@@ -56,93 +55,3 @@ class GetExceptionHandlerTests(TestCase):
         for exception in INVALID_EXCEPTIONS:
         for exception in INVALID_EXCEPTIONS:
             with self.assertRaises(ValueError):
             with self.assertRaises(ValueError):
                 exceptionhandler.get_exception_handler(exception())
                 exceptionhandler.get_exception_handler(exception())
-
-
-class HandleHttp404ExceptionTests(TestCase):
-    def setUp(self):
-        self.exception = Http404()
-        self.request = RequestFactory().get(reverse('forum_index'))
-
-    def test_get_handle_http404_exception(self):
-        """get_exception_handler returns correct Http404 exception handler"""
-        found_handler = exceptionhandler.get_exception_handler(self.exception)
-        self.assertEqual(
-            found_handler, exceptionhandler.handle_http404_exception)
-
-        response = found_handler(self.request, self.exception)
-        self.assertEqual(response.status_code, 404)
-
-    def test_handle_http404_exception(self):
-        """handle_misago_exception handles Http404 exception correctly"""
-        response = exceptionhandler.handle_misago_exception(
-            self.request, self.exception)
-        self.assertEqual(response.status_code, 404)
-
-
-class HandleOutdatedSlugExceptionTests(TestCase):
-    def setUp(self):
-        self.exception = OutdatedSlug()
-        self.request = RequestFactory().get(reverse('forum_index'))
-
-    def test_get_handle_outdated_slug_exception(self):
-        """
-        get_exception_handler returns correct OutdatedSlug exception handler
-        """
-        found_handler = exceptionhandler.get_exception_handler(self.exception)
-        self.assertEqual(
-            found_handler, exceptionhandler.handle_outdated_slug_exception)
-
-        response = found_handler(self.request, self.exception)
-        self.assertEqual(response.status_code, 301)
-
-    def test_handle_outdated_slug_exception(self):
-        """
-        handle_misago_exception handles OutdatedSlug exception correctly
-        """
-        response = exceptionhandler.handle_misago_exception(
-            self.request, self.exception)
-        self.assertEqual(response.status_code, 301)
-
-
-class HandlePermissionDeniedExceptionTests(TestCase):
-    def setUp(self):
-        self.exception_message = "Page access not allowed"
-        self.exception = PermissionDenied(self.exception_message)
-        self.request = RequestFactory().get(reverse('forum_index'))
-
-    def test_get_handle_permission_denied_exception(self):
-        """
-        get_exception_handler returns correct PermissionDenied exception
-        handler
-        """
-        found_handler = exceptionhandler.get_exception_handler(self.exception)
-        self.assertEqual(
-            found_handler, exceptionhandler.handle_permission_denied_exception)
-
-        response = found_handler(self.request, self.exception)
-        self.assertIn(self.exception_message, response.content)
-        self.assertEqual(response.status_code, 403)
-
-    def test_handle_permission_denied_exception(self):
-        """
-        handle_misago_exception handles PermissionDenied exception correctly
-        """
-        response = exceptionhandler.handle_misago_exception(
-            self.request, self.exception)
-        self.assertIn(self.exception_message, response.content)
-        self.assertEqual(response.status_code, 403)
-
-
-class GetExceptionMessageTests(TestCase):
-    def test_get_exception_message(self):
-        """_get_exception_message returns message for exception with one"""
-        message = "Fish's name Eric"
-        exception = Http404(message)
-        self.assertEqual(exceptionhandler._get_exception_message(exception),
-                         message)
-
-    def test_get_no_exception_message(self):
-        """_get_exception_message returns None for exception without one"""
-        exception = Http404()
-        self.assertEqual(exceptionhandler._get_exception_message(exception),
-                         None)

+ 1 - 1
misago/core/tests/test_middleware_exceptionhandler.py

@@ -7,7 +7,7 @@ from misago.core.middleware.exceptionhandler import ExceptionHandlerMiddleware
 
 
 class ExceptionHandlerMiddlewareTests(TestCase):
 class ExceptionHandlerMiddlewareTests(TestCase):
     def setUp(self):
     def setUp(self):
-        self.request = RequestFactory().get(reverse('forum_index'))
+        self.request = RequestFactory().get(reverse('misago:index'))
 
 
     def test_middleware_returns_response_for_supported_exception(self):
     def test_middleware_returns_response_for_supported_exception(self):
         """Middleware returns HttpResponse for supported exception"""
         """Middleware returns HttpResponse for supported exception"""

+ 30 - 13
misago/core/tests/test_shortcuts.py

@@ -1,37 +1,54 @@
+from django.core.urlresolvers import reverse
 from django.test import TestCase
 from django.test import TestCase
-from misago.core.shortcuts import check_object_slug, OutdatedSlug
+from misago.core.shortcuts import validate_slug, OutdatedSlug
+from misago.core.testproject.models import Model
 
 
 
 
-class MockModel(object):
-    def __init__(self, slug):
-        self.slug = slug
-
-
-class CheckObjectSlugTests(TestCase):
+class ValidateSlugTests(TestCase):
     def test_is_outdated_slug_exception_not_raised_for_valid_slug(self):
     def test_is_outdated_slug_exception_not_raised_for_valid_slug(self):
         """
         """
         check_object_slug doesn't raise OutdatedSlug when slugs match
         check_object_slug doesn't raise OutdatedSlug when slugs match
         """
         """
-        model = MockModel("test-slug")
-        check_object_slug(model, "test-slug")
+        model = Model(1, "test-slug")
+        validate_slug(model, "test-slug")
 
 
     def test_is_outdated_slug_exception_raised_for_invalid_slug(self):
     def test_is_outdated_slug_exception_raised_for_invalid_slug(self):
         """
         """
         check_object_slug raises OutdatedSlug when slugs mismatch
         check_object_slug raises OutdatedSlug when slugs mismatch
         """
         """
-        model = MockModel("test-slug")
+        model = Model(1, "test-slug")
 
 
         with self.assertRaises(OutdatedSlug):
         with self.assertRaises(OutdatedSlug):
-            check_object_slug(model, "wrong-slug")
+            validate_slug(model, "wrong-slug")
 
 
     def test_is_outdated_slug_exception_raised_with_valid_message(self):
     def test_is_outdated_slug_exception_raised_with_valid_message(self):
         """
         """
         check_object_slug raises OutdatedSlug with valid message
         check_object_slug raises OutdatedSlug with valid message
         """
         """
         correct_slug = "test-slug"
         correct_slug = "test-slug"
-        model = MockModel(correct_slug)
+        model = Model(1, correct_slug)
 
 
         try:
         try:
-            check_object_slug(model, "wrong-slug")
+            validate_slug(model, "wrong-slug")
         except OutdatedSlug as e:
         except OutdatedSlug as e:
             self.assertEqual(model, e.args[0])
             self.assertEqual(model, e.args[0])
+
+
+class CheckSlugHandler(TestCase):
+    urls = 'misago.core.testproject.urls'
+
+    def test_valid_slug_handle(self):
+        """valid slug causes no interruption in view processing"""
+        test_kwargs = {'model_slug': 'eric-the-fish', 'model_id': 1}
+        response = self.client.get(
+            reverse('validate_slug_view', kwargs=test_kwargs))
+        self.assertIn("Allright", response.content)
+
+    def test_invalid_slug_handle(self):
+        """invalid slug returns in redirect to valid page"""
+        test_kwargs = {'model_slug': 'lion-the-eric', 'model_id': 1}
+        response = self.client.get(
+            reverse('validate_slug_view', kwargs=test_kwargs))
+
+        valid_url = "http://testserver/forum/test-valid-slug/eric-the-fish-1/"
+        self.assertEqual(response['Location'], valid_url)

+ 1 - 1
misago/core/tests/test_threadstore.py

@@ -28,7 +28,7 @@ class ThreadStoreTests(TestCase):
 
 
 class ThreadStoreMiddlewareTests(TestCase):
 class ThreadStoreMiddlewareTests(TestCase):
     def setUp(self):
     def setUp(self):
-        self.request = RequestFactory().get(reverse('forum_index'))
+        self.request = RequestFactory().get(reverse('misago:index'))
 
 
     def test_middleware_clears_store_on_response_exception(self):
     def test_middleware_clears_store_on_response_exception(self):
         """Middleware cleared store on response"""
         """Middleware cleared store on response"""

+ 1 - 1
misago/core/tests/test_utils.py

@@ -20,7 +20,7 @@ class IsRequestToMisagoTests(TestCase):
         """
         """
         is_request_to_misago correctly detects requests directed at Misago
         is_request_to_misago correctly detects requests directed at Misago
         """
         """
-        misago_prefix = reverse('forum_index')
+        misago_prefix = reverse('misago:index')
 
 
         for path in VALID_PATHS:
         for path in VALID_PATHS:
             request = RequestFactory().get('/')
             request = RequestFactory().get('/')

+ 1 - 1
misago/core/tests/test_views.py

@@ -5,5 +5,5 @@ from django.test import TestCase
 class ForumIndexViewTests(TestCase):
 class ForumIndexViewTests(TestCase):
     def test_forum_index_returns_200(self):
     def test_forum_index_returns_200(self):
         """forum_index view has no show-stoppers"""
         """forum_index view has no show-stoppers"""
-        response = self.client.get(reverse('forum_index'))
+        response = self.client.get(reverse('misago:index'))
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.status_code, 200)

+ 1 - 1
misago/core/utils.py

@@ -3,7 +3,7 @@ from django.core.urlresolvers import reverse
 
 
 def _is_request_path_under_misago(request):
 def _is_request_path_under_misago(request):
     # We are assuming that forum_index link is root of all Misago links
     # We are assuming that forum_index link is root of all Misago links
-    forum_index = reverse('forum_index')
+    forum_index = reverse('misago:index')
     path_info = request.path_info
     path_info = request.path_info
 
 
     if len(forum_index) > len(path_info):
     if len(forum_index) > len(path_info):

+ 1 - 1
misago/project_template/project_name/urls.py

@@ -5,7 +5,7 @@ from django.contrib import admin
 admin.autodiscover()
 admin.autodiscover()
 
 
 urlpatterns = patterns('',
 urlpatterns = patterns('',
-    url(r'^', include('misago.urls')),
+    url(r'^', include('misago.urls', namespace='misago')),
     # Uncomment next line if you plan to use Django admin for 3rd party apps
     # Uncomment next line if you plan to use Django admin for 3rd party apps
     #url(r'^django-admin/', include(admin.site.urls)),
     #url(r'^django-admin/', include(admin.site.urls)),
 )
 )

+ 1 - 1
misago/urls.py

@@ -6,5 +6,5 @@ urlpatterns = patterns('misago.core',
     # any request with path that falls below this one is assumed to be directed
     # any request with path that falls below this one is assumed to be directed
     # at Misago and will be handled by misago.views.exceptionhandler if it
     # at Misago and will be handled by misago.views.exceptionhandler if it
     # results in Http404 or PermissionDenied exception
     # results in Http404 or PermissionDenied exception
-    url(r'^$', 'views.forum_index', name='forum_index'),
+    url(r'^$', 'views.forum_index', name='index'),
 )
 )