Convert place views to class-based views

letterpress/urls.py

@@ -5,8 +5,8 @@
 from django.conf.urls import include, url
 from django.contrib.auth import views as auth_views
 from letters.views import GetStatsView, GetTextSentimentView, GetWordCloudView, LetterDetailView, \
-    LetterSentimentView, LettersView, logout_view, place_by_id, PlaceListView, RandomLetterView, SearchView, \
-    search_places, SentimentView, StatsView, TextSentimentView, WordCloudView
+    LetterSentimentView, LettersView, logout_view, PlaceDetailView, PlaceListView, PlaceSearchView, RandomLetterView, \
+    SearchView, SentimentView, StatsView, TextSentimentView, WordCloudView
 from letterpress.views import HomeView
 
 from django.contrib import admin
@@ -22,15 +22,15 @@
                         LetterSentimentView.as_view(), name='letter_sentiment_view'),
                   url(r'^letters/(?P<pk>[0-9]+)/$', LetterDetailView.as_view(), name='letter_detail'),
                   url(r'^letters/', LettersView.as_view(), name='letters_view'),
-                  url(r'^search_places/', search_places, name='search_places'),
                   url(r'^search/', SearchView.as_view(), name='search'),
                   url(r'^random_letter/', RandomLetterView.as_view(), name='random_letter'),
                   url(r'^stats/', StatsView.as_view(), name='stats_view'),
                   url(r'^get_stats/', GetStatsView.as_view(), name='get_stats'),
                   url(r'^sentiment/', SentimentView.as_view(), name='sentiment_view'),
                   url(r'^text_sentiment/', TextSentimentView.as_view(), name='text_sentiment_view'),
                   url(r'^get_text_sentiment/', GetTextSentimentView.as_view(), name='get_text_sentiment'),
-                  url(r'^places/(?P<place_id>[0-9]+)/$', place_by_id, name='place_by_id'),
+                  url(r'^places/search/', PlaceSearchView.as_view(), name='place_search'),
+                  url(r'^places/(?P<pk>[0-9]+)/$', PlaceDetailView.as_view(), name='place_detail'),
                   url(r'^places/', PlaceListView.as_view(), name='place_list'),
                   url(r'^tinymce/', include('tinymce.urls')),
                   url(r'^wordcloud_image.png', GetWordCloudView.as_view(), name='get_wordcloud'),

letters/static/js/place_search.js

@@ -30,7 +30,7 @@ function do_place_search() {
             end_date: inital_filter_values.end_date,
             page_number: 0,
         },
-        url: "/search_places/",
+        url: "/places/search/",
         success: function (result) {
             show_map(result.map)
         }

letters/tests/test_views.py

@@ -14,9 +14,9 @@
 
 from letters.models import Correspondent, Letter
 from letters.tests.factories import LetterFactory, PlaceFactory
-from letters.views import export_csv, export_text, get_letter_export_text, GetStatsView, GetTextSentimentView, \
-    GetWordCloudView, highlight_for_sentiment, highlight_letter_for_sentiment, LettersView, RandomLetterView, \
-    SearchView, search_places, show_letter_content, get_highlighted_letter_sentiment
+from letters.views import export_csv, export_text, get_highlighted_letter_sentiment, get_letter_export_text, \
+    GetStatsView, GetTextSentimentView, GetWordCloudView, highlight_for_sentiment, highlight_letter_for_sentiment, \
+    LettersView, PlaceSearchView, RandomLetterView, SearchView, show_letter_content
 
 
 class LettersViewTestCase(TestCase):
@@ -827,64 +827,63 @@ def test_place_list_view(self, mock_render_to_string, mock_get_initial_filter_va
         self.assertTrue('<title>Places</title>' in content, "PlaceListView should return page with 'Places' as title")
 
 
-class SearchPlacesTestCase(SimpleTestCase):
+class PlaceSearchViewTestCase(SimpleTestCase):
     """
-    Test search_places()
+    Test PlaceSearchView
     """
 
     @patch('letters.views.letter_search.do_letter_search')
     @patch('letters.views.render_to_string', autospec=True)
-    def test_search_places(self, mock_render_to_string, mock_do_letter_search):
-        # GET request should raise ValueError
-        with self.assertRaises(ValueError):
-            self.client.get(reverse('search_places'), follow=True)
+    def test_place_search_view(self, mock_render_to_string, mock_do_letter_search):
+        # GET request should return HttpResponseNotAllowed
+        response = self.client.get(reverse('place_search'), follow=True)
+        self.assertEqual(response.status_code, 405,
+                         'Making a GET request to PlaceSearchView should return HttpResponseNotAllowed')
 
         mock_render_to_string.return_value = 'render to string'
 
         # POST
         # For some reason, it's impossible to request a POST request via the Django test client,
         # so manually create one and call the view directly
-        request = RequestFactory().post(reverse('search_places'))
+        request = RequestFactory().post(reverse('place_search'))
 
-        response = search_places(request)
+        response = PlaceSearchView().dispatch(request)
 
-        self.assertEqual(mock_do_letter_search.call_count, 1, 'search_places() should call do_letter_search()')
-        self.assertEqual(mock_render_to_string.call_count, 1, 'search_places() should call render_to_string()')
+        self.assertEqual(mock_do_letter_search.call_count, 1, 'PlaceSearchView should call do_letter_search()')
+        self.assertEqual(mock_render_to_string.call_count, 1, 'PlaceSearchView should call render_to_string()')
 
         content = json.loads(response.content.decode('utf-8'))
         self.assertEqual(content['map'], mock_render_to_string.return_value,
-                         "search_places() should return response containing 'map'")
+                         "PlaceSearchView should return response containing 'map'")
 
 
-class PlaceByIdTestCase(TestCase):
+class PlaceDetailViewTestCase(TestCase):
     """
-    Test place_by_id()
+    Test PlaceDetailView
     """
 
-    def test_place_by_id(self):
+    def test_place_detail_view(self):
         """
-        place_by_id(request, place_id) should return rendered html if Place with id found
-        Otherwise it should return object_not_found()
-
-        For some reason, object_not_found() can't be successfully mocked, so actually call it
+        PlaceDetailView should return rendered html if Place with id found
+        Otherwise it should return object not found
         """
 
-        response = self.client.get(reverse('place_by_id', kwargs={'place_id': '1'}), follow=True)
+        response = self.client.get(reverse('place_detail', kwargs={'pk': '1'}), follow=True)
 
         expected = {'title': 'Place not found', 'object_id': '1', 'object_type': 'Place'}
         for key in expected.keys():
             self.assertEqual(response.context[key], expected[key],
-                "place_by_id() context '{}' should be '{}', if place not found".format(key, expected[key]))
+                "PlaceDetailView context '{}' should be '{}', if place not found".format(key, expected[key]))
 
 
-        # If Place with place_id found, place_by_id() should return render()
-        response = self.client.get(reverse('place_by_id', kwargs={'place_id': PlaceFactory().pk}), follow=True)
+        # If Place with pk found, PlaceDetailView should return render()
+        response = self.client.get(reverse('place_detail', kwargs={'pk': PlaceFactory().pk}), follow=True)
         self.assertTemplateUsed(response, 'place.html')
 
         expected = {'title': 'Place', 'nbar': 'places'}
         for key in expected.keys():
             self.assertEqual(response.context[key], expected[key],
-                "place_by_id() context '{}' should be '{}', if letter found".format(key, expected[key]))
+                "PlaceDetailView context '{}' should be '{}', if letter found".format(key, expected[key]))
 
 
 class LogoutViewTestCase(SimpleTestCase):

letters/views.py

@@ -385,6 +385,7 @@ class LetterDetailView(DetailView, ObjectNotFoundMixin):
     """
 
     model = Letter
+    context_object_name = 'letter'
     template_name = 'letter.html'
 
     def get_context_data(self, **kwargs):
@@ -394,7 +395,6 @@ def get_context_data(self, **kwargs):
         letter.body = mark_safe(letter.body)
         context['title'] = 'Letter'
         context['nbar'] = 'letters_view'
-        context['letter'] = letter
 
         return context
 
@@ -489,34 +489,43 @@ def get_context_data(self, **kwargs):
         return context
 
 
-# return map of places whose letters meet search criteria
-def search_places(request):
-    assert isinstance(request, HttpRequest)
-    if request.method != 'POST':
-        return
-    # get a bunch of them!
-    size = 5000
-    # Search for letters that meet criteria. Start at beginning, so page number = 0
-    es_result = letter_search.do_letter_search(request, size, page_number=0)
-    # Get list of corresponding places
-    place_ids = set([letter.place_id for letter, highlight, sentiments, score in es_result.search_results])
-    # Only show the first 100
-    places = Place.objects.filter(pk__in=place_ids, point__isnull=False)[:100]
-    map_html = render_to_string('snippets/map.html', {'places': places})
-    # This was Ajax
-    return HttpResponse(json.dumps({'map': map_html}), content_type="application/json")
-
-
-# view to show one place by id
-def place_by_id(request, place_id):
-    assert isinstance(request, HttpRequest)
-    try:
-        place = Place.objects.get(pk=place_id)
-        letters = Letter.objects.filter(place=place_id).order_by('date')
-        return render(request, 'place.html',
-                      {'title': 'Place', 'nbar': 'places', 'place': place, 'letters': letters})
-    except Place.DoesNotExist:
-        return object_not_found(request, place_id, 'Place')
+class PlaceSearchView(View):
+    """
+    Return map of places whose letters meet search criteria
+    """
+
+    def post(self, request, *args, **kwargs):
+        # get a bunch of them!
+        size = 5000
+        # Search for letters that meet criteria. Start at beginning, so page number = 0
+        es_result = letter_search.do_letter_search(request, size, page_number=0)
+        # Get list of corresponding places
+        place_ids = set([letter.place_id for letter, highlight, sentiments, score in es_result.search_results])
+        # Only show the first 100
+        places = Place.objects.filter(pk__in=place_ids, point__isnull=False)[:100]
+        map_html = render_to_string('snippets/map.html', {'places': places})
+        # This was Ajax
+        return HttpResponse(json.dumps({'map': map_html}), content_type="application/json")
+
+
+class PlaceDetailView(DetailView, ObjectNotFoundMixin):
+    """
+    Show one place by id
+    """
+
+    model = Place
+    context_object_name = 'place'
+    template_name = 'place.html'
+
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+
+        letters = Letter.objects.filter(place=self.object).order_by('date')
+        context['title'] = 'Place'
+        context['nbar'] = 'places'
+        context['letters'] = letters
+
+        return context
 
 
 def logout_view(request):

templates/place.html

@@ -60,7 +60,7 @@ <h6>Coordinates: {{ place.point.x }}, {{ place.point.y }}</h6>
 
         // Define markers as "features" of the vector layer:
         var point = create_point({{ place.point.x }}, {{ place.point.y }});
-        var name = '<a href ="{% url 'place_by_id' place.id %}">{{ place }}</a>';
+        var name = '<a href ="{% url 'place_detail' place.pk %}">{{ place }}</a>';
 
         features.push(create_feature(point, name));
 

templates/snippets/map.html

@@ -27,7 +27,7 @@
             show_place_alert("{{ place }}", {{ place.point.x }}, {{ place.point.y }});
         }
         else {
-            var name = '<a href ="{% url 'place_by_id' place.id %}">{{ place }}</a>';
+            var name = '<a href ="{% url 'place_detail' place.pk %}">{{ place }}</a>';
             features.push(create_feature(point, name));
         }