Merge pull request #157 from akx/fix-comment-endpoint

Fix the comments/ endpoint

democracy/tests/test_comment.py

@@ -11,7 +11,7 @@
 from democracy.models.section import SectionComment
 from democracy.tests.conftest import default_comment_content, green_comment_content, red_comment_content
 from democracy.tests.test_images import get_hearing_detail_url
-from democracy.tests.utils import assert_datetime_fuzzy_equal, get_data_from_response
+from democracy.tests.utils import assert_datetime_fuzzy_equal, get_data_from_response, assert_common_keys_equal
 
 
 def get_comment_data(**extra):
@@ -150,6 +150,7 @@ def test_56_add_comment_to_section_without_data(api_client, default_hearing):
 def test_56_add_comment_to_section(john_doe_api_client, default_hearing):
     section = default_hearing.sections.first()
     url = get_hearing_detail_url(default_hearing.id, 'sections/%s/comments' % section.id)
+    old_comment_list = get_data_from_response(john_doe_api_client.get(url))
 
     # set section explicitly
     comment_data = get_comment_data(section=section.pk)
@@ -162,6 +163,18 @@ def test_56_add_comment_to_section(john_doe_api_client, default_hearing):
     assert 'content' in data
     assert data['content'] == default_comment_content
 
+    # Check that the comment is available in the comment endpoint now
+    new_comment_list = get_data_from_response(john_doe_api_client.get(url))
+    assert len(new_comment_list) == len(old_comment_list) + 1
+    new_comment = [c for c in new_comment_list if c["id"] == data["id"]][0]
+    assert_common_keys_equal(new_comment, comment_data)
+    assert_common_keys_equal(new_comment["created_by"], {
+        "first_name": john_doe_api_client.user.first_name,
+        "last_name": john_doe_api_client.user.last_name,
+        "username": john_doe_api_client.user.username,
+    })
+
+
 
 @pytest.mark.django_db
 def test_56_get_hearing_with_section_check_n_comments_property(api_client):

democracy/tests/utils.py

@@ -85,3 +85,8 @@ def get_geojson():
 def assert_id_in_results(id, results, expected=True):
     included = id in [value['id'] for value in results]
     assert included is expected
+
+
+def assert_common_keys_equal(dict1, dict2):
+    for key in set(dict1) & set(dict2):
+        assert dict1[key] == dict2[key]

democracy/views/base.py

@@ -1,3 +1,4 @@
+from django.contrib.auth import get_user_model
 from django.core.exceptions import ImproperlyConfigured
 from rest_framework import serializers
 
@@ -8,6 +9,9 @@
 
 class UserFieldSerializer(serializers.ModelSerializer):
 
+    class Meta:
+        model = get_user_model()
+
     def to_representation(self, user):
         return {
             "uuid": user.uuid,

democracy/views/hearing.py

@@ -13,6 +13,7 @@
 from democracy.views.hearing_comment import HearingCommentSerializer
 from democracy.views.label import LabelSerializer
 from democracy.views.section import SectionFieldSerializer
+from democracy.views.utils import IOErrorIgnoringManyRelatedField
 
 from .hearing_report import HearingReport
 
@@ -42,7 +43,10 @@ def get_queryset(self):
 
 class HearingSerializer(serializers.ModelSerializer):
     labels = LabelSerializer(many=True, read_only=True)
-    images = HearingImageSerializer.get_field_serializer(many=True, read_only=True)
+    images = HearingImageSerializer.get_field_serializer(
+        many=True, read_only=True,
+        many_field_class=IOErrorIgnoringManyRelatedField
+    )
     sections = serializers.SerializerMethodField()
     comments = HearingCommentSerializer.get_field_serializer(many=True, read_only=True)
     commenting = EnumField(enum_type=Commenting)

democracy/views/section.py

@@ -4,10 +4,10 @@
 from democracy.models import Hearing, Section, SectionImage
 from democracy.utils.drf_enum_field import EnumField
 from democracy.views.base import AdminsSeeUnpublishedMixin, BaseImageSerializer
+from democracy.views.utils import IOErrorIgnoringManyRelatedField
 
 
 class SectionImageSerializer(BaseImageSerializer):
-
     class Meta:
         model = SectionImage
         fields = ['title', 'url', 'width', 'height', 'caption']
@@ -17,7 +17,9 @@ class SectionSerializer(serializers.ModelSerializer):
     """
     Serializer for section instance.
     """
-    images = SectionImageSerializer.get_field_serializer(many=True, read_only=True)
+    images = SectionImageSerializer.get_field_serializer(
+        many=True, read_only=True, many_field_class=IOErrorIgnoringManyRelatedField
+    )
     type = serializers.SlugRelatedField(slug_field='identifier', read_only=True)
     type_name_singular = serializers.SlugRelatedField(source='type', slug_field='name_singular', read_only=True)
     type_name_plural = serializers.SlugRelatedField(source='type', slug_field='name_plural', read_only=True)

democracy/views/utils.py

@@ -1,25 +1,61 @@
 # -*- coding: utf-8 -*-
 from functools import lru_cache
 
+from django.db.models.query import QuerySet
 from rest_framework import serializers
+from rest_framework.relations import ManyRelatedField, MANY_RELATION_KWARGS
 
 
 class AbstractFieldSerializer(serializers.RelatedField):
     parent_serializer_class = serializers.ModelSerializer
+    many_field_class = ManyRelatedField
 
     def to_representation(self, image):
         return self.parent_serializer_class(image, context=self.context).data
 
+    @classmethod
+    def many_init(cls, *args, **kwargs):
+        list_kwargs = {'child_relation': cls(*args, **kwargs)}
+        for key in kwargs.keys():
+            if key in MANY_RELATION_KWARGS:
+                list_kwargs[key] = kwargs[key]
+        return cls.many_field_class(**list_kwargs)
+
 
 class AbstractSerializerMixin(object):
 
     @classmethod
     @lru_cache()
-    def get_field_serializer_class(cls):
+    def get_field_serializer_class(cls, many_field_class=ManyRelatedField):
         return type('%sFieldSerializer' % cls.Meta.model, (AbstractFieldSerializer,), {
-            "parent_serializer_class": cls
+            "parent_serializer_class": cls,
+            "many_field_class": many_field_class,
         })
 
     @classmethod
     def get_field_serializer(cls, **kwargs):
-        return cls.get_field_serializer_class()(**kwargs)
+        many_field_class = kwargs.pop("many_field_class", ManyRelatedField)
+        return cls.get_field_serializer_class(many_field_class=many_field_class)(**kwargs)
+
+
+class IOErrorIgnoringManyRelatedField(ManyRelatedField):
+    """
+    A ManyRelatedField that ignores IOErrors occurring during iterating the children.
+
+    This is mainly useful for images that are referenced in the database but do not exist
+    on the server (where constructing them requires accessing them to populate the width
+    and height fields).
+    """
+    def to_representation(self, iterable):
+        out = []
+        if isinstance(iterable, QuerySet):
+            iterable = iterable.iterator()
+        while True:
+            try:
+                value = next(iterable)
+                out.append(self.child_relation.to_representation(value))
+            except StopIteration:
+                break
+            except IOError:
+                continue
+        return out