Question URL mapping (old -> new) + some cleanup of question-related redirects + added some tests for redirects

4 files changed, 130 insertions, 14 deletions

diff --git a/askbot/forms.py b/askbot/forms.py
index 34bb7f4f..4e490ce6 100644
--- a/askbot/forms.py
+++ b/askbot/forms.py
@@ -334,13 +334,11 @@ class ShowQuestionForm(forms.Form):
         in_data = self.get_pruned_data()
         out_data = dict()
         if ('answer' in in_data) ^ ('comment' in in_data):
-            out_data['is_permalink'] = True
             out_data['show_page'] = None
             out_data['answer_sort_method'] = 'votes'
             out_data['show_comment'] = in_data.get('comment', None)
             out_data['show_answer'] = in_data.get('answer', None)
         else:
-            out_data['is_permalink'] = False
             out_data['show_page'] = in_data.get('page', 1)
             out_data['answer_sort_method'] = in_data.get(
                                                     'sort',
diff --git a/askbot/skins/default/templates/question.html b/askbot/skins/default/templates/question.html
index 2ec16927..c809b3dc 100644
--- a/askbot/skins/default/templates/question.html
+++ b/askbot/skins/default/templates/question.html
@@ -153,6 +153,10 @@
             {% for answer in answers %}
                 {# ==== START: question/answer_card.html ==== #}
                 <a name="{{ answer.id }}"></a>
+                {% if answer.old_answer_id %}
+                    {# Make old URL anchors/hashes work #}
+                    <a class="old_answer_id_anchor" name="{{ answer.old_answer_id }}"></a>
+                {% endif %}
                 <div
                         id="answer-container-{{ answer.id }}"
                         class="{{ macros.answer_classes(answer, question) }}">
diff --git a/askbot/tests/page_load_tests.py b/askbot/tests/page_load_tests.py
index be58da4b..ac84d092 100644
--- a/askbot/tests/page_load_tests.py
+++ b/askbot/tests/page_load_tests.py
@@ -488,3 +488,87 @@ class AvatarTests(AskbotTestCase):
                             'avatar_render_primary',
                             kwargs = {'user': 'john doe', 'size': 48}
                         )
+
+
+class QuestionPageRedirectTests(AskbotTestCase):
+
+    def setUp(self):
+        self.create_user()
+
+        self.q = self.post_question()
+        self.q.old_question_id = 101
+        self.q.save()
+
+        self.a = self.post_answer(question=self.q)
+        self.a.old_answer_id = 201
+        self.a.save()
+
+        self.c = self.post_comment(parent_post=self.a)
+        self.c.old_comment_id = 301
+        self.c.save()
+
+    def test_bare_question(self):
+        resp = self.client.get(self.q.get_absolute_url())
+        self.assertEqual(200, resp.status_code)
+        self.assertEqual(self.q, resp.context['question'])
+
+        url = reverse('question', kwargs={'id': self.q.id})
+        resp = self.client.get(url)
+        url = url + self.q.slug
+        self.assertRedirects(resp, expected_url=url)
+
+        resp = self.client.get(url)
+        self.assertEqual(200, resp.status_code)
+        self.assertEqual(self.q, resp.context['question'])
+
+        url = reverse('question', kwargs={'id': 101})
+        resp = self.client.get(url)
+        url = reverse('question', kwargs={'id': self.q.id}) + self.q.slug  # redirect uses the new question.id !
+        self.assertRedirects(resp, expected_url=url)
+
+        url = reverse('question', kwargs={'id': 101}) + self.q.slug
+        resp = self.client.get(url)
+        self.assertEqual(200, resp.status_code)
+        self.assertEqual(self.q, resp.context['question'])
+
+    def test_show_answer(self):
+        resp = self.client.get(self.a.get_absolute_url())
+        self.assertEqual(200, resp.status_code)
+        self.assertEqual(self.q, resp.context['question'])
+        self.assertEqual(self.a, resp.context['show_post'])
+
+        url = reverse('question', kwargs={'id': self.q.id})
+        resp = self.client.get(url, data={'answer': self.a.id})
+        url = url + self.q.slug
+        self.assertRedirects(resp, expected_url=url + '?answer=%d' % self.a.id)
+
+        resp = self.client.get(url, data={'answer': self.a.id})
+        self.assertEqual(200, resp.status_code)
+        self.assertEqual(self.q, resp.context['question'])
+        self.assertEqual(self.a, resp.context['show_post'])
+
+        url = reverse('question', kwargs={'id': 101}) + self.q.slug
+        resp = self.client.get(url, data={'answer': 201})
+        self.assertRedirects(resp, expected_url=self.a.get_absolute_url())
+
+    def test_show_comment(self):
+        resp = self.client.get(self.c.get_absolute_url())
+        self.assertEqual(200, resp.status_code)
+        self.assertEqual(self.q, resp.context['question'])
+        self.assertEqual(self.a, resp.context['show_post'])
+        self.assertEqual(self.c, resp.context['show_comment'])
+
+        url = reverse('question', kwargs={'id': self.q.id})
+        resp = self.client.get(url, data={'comment': self.c.id})
+        url = url + self.q.slug
+        self.assertRedirects(resp, expected_url=url + '?comment=%d' % self.c.id)
+
+        resp = self.client.get(url, data={'comment': self.c.id})
+        self.assertEqual(200, resp.status_code)
+        self.assertEqual(self.q, resp.context['question'])
+        self.assertEqual(self.a, resp.context['show_post'])
+        self.assertEqual(self.c, resp.context['show_comment'])
+
+        url = reverse('question', kwargs={'id': 101}) + self.q.slug
+        resp = self.client.get(url, data={'comment': 301})
+        self.assertRedirects(resp, expected_url=self.c.get_absolute_url())
diff --git a/askbot/views/readers.py b/askbot/views/readers.py
index 265d51ea..edcf2436 100644
--- a/askbot/views/readers.py
+++ b/askbot/views/readers.py
@@ -369,9 +369,35 @@ def question(request, id):#refactor - long subroutine. display question body, an
     show_answer = form.cleaned_data['show_answer']
     show_comment = form.cleaned_data['show_comment']
     show_page = form.cleaned_data['show_page']
-    is_permalink = form.cleaned_data['is_permalink']
     answer_sort_method = form.cleaned_data['answer_sort_method']
 
+    # Handle URL mapping - from old Q/A/C/ URLs to the new one
+    if not models.Post.objects.get_questions().filter(id=id).exists() and models.Post.objects.get_questions().filter(old_question_id=id).exists():
+        old_question = models.Post.objects.get_questions().get(old_question_id=id)
+
+        # If we are supposed to show a specific answer or comment, then just redirect to the new URL...
+        if show_answer:
+            try:
+                old_answer = models.Post.objects.get_answers().get(old_answer_id=show_answer)
+                return HttpResponseRedirect(old_answer.get_absolute_url())
+            except models.Post.DoesNotExist:
+                pass
+
+        elif show_comment:
+            try:
+                old_comment = models.Post.objects.get_comments().get(old_comment_id=show_comment)
+                return HttpResponseRedirect(old_comment.get_absolute_url())
+            except models.Post.DoesNotExist:
+                pass
+
+        # ...otherwise just patch question.id, to make URLs like this one work: /question/123#345
+        # This is because URL fragment (hash) (i.e. #345) is not passed to the server so we can't know which
+        # answer user expects to see. If we made a redirect to the new question.id then that hash would be lost.
+        # And if we just hack the question.id (and in question.html template /or its subtemplate/ we create anchors for both old and new id-s)
+        # then everything should work as expected.
+        id = old_question.id
+
+
     #resolve comment and answer permalinks
     #they go first because in theory both can be moved to another question
     #this block "returns" show_post and assigns actual comment and answer
@@ -379,7 +405,7 @@ def question(request, id):#refactor - long subroutine. display question body, an
     #in the case if the permalinked items or their parents are gone - redirect
     #redirect also happens if id of the object's origin post != requested id
     show_post = None #used for permalinks
-    if show_comment is not None:
+    if show_comment:
         #if url calls for display of a specific comment,
         #check that comment exists, that it belongs to
         #the current question
@@ -389,18 +415,21 @@ def question(request, id):#refactor - long subroutine. display question body, an
         #in addition - if url points to a comment and the comment
         #is for the answer - we need the answer object
         try:
-            show_comment = models.Post.objects.get(id=show_comment)
-            if str(show_comment.thread._question_post().id) != id:
-                return HttpResponseRedirect(show_comment.thread.get_absolute_url())
-            show_post = show_comment.parent
-            show_comment.assert_is_visible_to(request.user)
-        except models.Comment.DoesNotExist:
+            show_comment = models.Post.objects.get_comments().get(id=show_comment)
+        except models.Post.DoesNotExist:
             error_message = _(
                 'Sorry, the comment you are looking for has been '
                 'deleted and is no longer accessible'
             )
             request.user.message_set.create(message = error_message)
             return HttpResponseRedirect(reverse('question', kwargs = {'id': id}))
+
+        if str(show_comment.thread._question_post().id) != str(id):
+            return HttpResponseRedirect(show_comment.get_absolute_url())
+        show_post = show_comment.parent
+
+        try:
+            show_comment.assert_is_visible_to(request.user)
         except exceptions.AnswerHidden, error:
             request.user.message_set.create(message = unicode(error))
             #use reverse function here because question is not yet loaded
@@ -409,15 +438,16 @@ def question(request, id):#refactor - long subroutine. display question body, an
             request.user.message_set.create(message = unicode(error))
             return HttpResponseRedirect(reverse('index'))
 
-    elif show_answer is not None:
+    elif show_answer:
         #if the url calls to view a particular answer to 
         #question - we must check whether the question exists
         #whether answer is actually corresponding to the current question
         #and that the visitor is allowed to see it
+        show_post = get_object_or_404(models.Post, post_type='answer', id=show_answer)
+        if str(show_post.thread._question_post().id) != str(id):
+            return HttpResponseRedirect(show_post.get_absolute_url())
+
         try:
-            show_post = get_object_or_404(models.Post, id=show_answer)
-            if str(show_post.thread._question_post().id) != id:
-                return HttpResponseRedirect(show_post.get_absolute_url())
             show_post.assert_is_visible_to(request.user)
         except django_exceptions.PermissionDenied, error:
             request.user.message_set.create(message = unicode(error))