Merge pull request #165 from willemarcel/develop

add view to list comments of a changeset

README.rst

@@ -60,6 +60,7 @@ DJANGO_NON_STAFF_USER_THROTTLE_RATE     NON_STAFF_USER_THROTTLE_RATE      3/min
 OAUTH_REDIRECT_URI                      OAUTH_REDIRECT_URI                http://localhost:8000/oauth-landing.html  http://localhost:8000/oauth-landing.html
 OSMCHA_FRONTEND_VERSION                 OSMCHA_FRONTEND_VERSION           oh-pages                                  oh-pages
 DJANGO_ENABLE_CHANGESET_COMMENTS        ENABLE_POST_CHANGESET_COMMENTS    False                                     False
+DJANGO_OSM_COMMENTS_API_KEY             OSM_COMMENTS_API_KEY              ''                                        ''
 ======================================= ================================= ========================================= ===========================================
 
 You can set each of these variables with:

config/settings/common.py

@@ -324,6 +324,9 @@
 # are reviewed
 ENABLE_POST_CHANGESET_COMMENTS = env('DJANGO_ENABLE_CHANGESET_COMMENTS', default=False)
 
+# Authorization token to access the OSM-COMMENTS-API
+OSM_COMMENTS_API_KEY = env('DJANGO_OSM_COMMENTS_API_KEY', default='')
+
 # Your common stuff: Below this line define 3rd party library settings
 REST_FRAMEWORK = {
     'DEFAULT_AUTHENTICATION_CLASSES': (

osmchadjango/changeset/tests/test_comment_views.py

@@ -8,7 +8,7 @@
 from social_django.models import UserSocialAuth
 from rest_framework.test import APITestCase
 from requests_oauthlib import OAuth1Session
-import mock
+from unittest import mock
 
 from ...users.models import User
 from ..models import Changeset
@@ -160,3 +160,53 @@ def test_comment_changeset_doesnt_exist(self):
             )
 
         self.assertEqual(response.status_code, 404)
+
+
+class TestChangesetCommentsView(APITestCase):
+    def setUp(self):
+        self.changeset = HarmfulChangesetFactory(
+            id=55736682,
+            )
+        HarmfulChangesetFactory(id=1343)
+        self.user = User.objects.create_user(
+            username='test',
+            password='password',
+            email='[email protected]',
+            )
+        UserSocialAuth.objects.create(
+            user=self.user,
+            provider='openstreetmap',
+            uid='123123',
+            )
+
+    def test_unauthenticated_changeset_detail_response(self):
+        response = self.client.get(
+            reverse('changeset:comment', args=[self.changeset.id])
+            )
+        self.assertEqual(response.status_code, 401)
+
+    def test_authenticated_changeset_detail_response(self):
+        self.client.login(username=self.user.username, password='password')
+        response = self.client.get(
+            reverse('changeset:comment', args=[self.changeset.id])
+            )
+
+        self.assertEqual(response.status_code, 200)
+        self.assertTrue(len(response.data) > 0)
+
+    def test_changeset_does_not_exist(self):
+        self.client.login(username=self.user.username, password='password')
+        response = self.client.get(
+            reverse('changeset:comment', args=[1234])
+            )
+
+        self.assertEqual(response.status_code, 404)
+
+    def test_changeset_without_comments(self):
+        self.client.login(username=self.user.username, password='password')
+        response = self.client.get(
+            reverse('changeset:comment', args=[1343])
+            )
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.data, [])

osmchadjango/changeset/tests/test_tasks.py

@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-from mock import patch
+from unittest.mock import patch
 from urllib.parse import quote
 
 from django.test import TestCase

osmchadjango/changeset/urls.py

@@ -72,7 +72,7 @@
     re_path(
         r'^changesets/(?P<pk>\w+)/comment/$',
         view=views.ChangesetCommentAPIView.as_view(
-            {'post': 'post_comment'}
+            {'post': 'post_comment', 'get': 'get_comments'}
             ),
         name='comment'
     ),

osmchadjango/changeset/views.py

@@ -24,6 +24,7 @@
 from rest_framework_gis.pagination import GeoJsonPagination
 from rest_framework_csv.renderers import CSVRenderer
 from rest_framework.exceptions import APIException
+import requests
 
 from .models import Changeset, UserWhitelist, SuspicionReasons, Tag
 from .filters import ChangesetFilter
@@ -465,13 +466,36 @@ def get_queryset(self):
 
 
 class ChangesetCommentAPIView(ModelViewSet):
-    """Post a comment to a changeset in the OpenStreetMap website."""
     queryset = Changeset.objects.all()
     permission_classes = (IsAuthenticated,)
     serializer_class = ChangesetCommentSerializer
 
+    @detail_route(methods=['get'])
+    def get_comments(self, request, pk):
+        "List comments received by a changeset on the OpenStreetMap website."
+        self.changeset = self.get_object()
+        headers = {
+            'apiKey': settings.OSM_COMMENTS_API_KEY,
+            'Content-Type': 'application/json'
+        }
+        data = requests.get(
+            'https://osm-comments-api.mapbox.com/api/v1/changesets/{}'.format(pk),
+            headers=headers
+            )
+        if data.status_code != 200:
+            return Response(
+                data.json(),
+                status=data.status_code
+                )
+        else:
+            return Response(
+                data.json().get('properties').get('comments'),
+                status=status.HTTP_200_OK
+                )
+
     @detail_route(methods=['post'])
     def post_comment(self, request, pk):
+        "Post a comment to a changeset in the OpenStreetMap website."
         self.changeset = self.get_object()
         serializer = self.get_serializer(data=request.data)
         if serializer.is_valid():

osmchadjango/supervise/migrations/0015_auto_20181129_1135.py

@@ -0,0 +1,24 @@
+# Generated by Django 2.0.9 on 2018-11-29 11:35
+
+from django.conf import settings
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('supervise', '0014_auto_20180307_1417'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='blacklisteduser',
+            name='uid',
+            field=models.CharField(max_length=255),
+        ),
+        migrations.AlterUniqueTogether(
+            name='blacklisteduser',
+            unique_together={('uid', 'added_by')},
+        ),
+    ]

osmchadjango/supervise/models.py

@@ -53,7 +53,7 @@ class Meta:
 
 class BlacklistedUser(models.Model):
     username = models.CharField(max_length=1000)
-    uid = models.CharField(max_length=255, unique=True)
+    uid = models.CharField(max_length=255)
     added_by = models.ForeignKey(User, on_delete=models.CASCADE)
     date = models.DateTimeField(auto_now_add=True)
 
@@ -65,4 +65,5 @@ def save(self, *args, **kwargs):
         super(BlacklistedUser, self).save(*args, **kwargs)
 
     class Meta:
+        unique_together = ('uid', 'added_by')
         ordering = ['-date']

osmchadjango/supervise/tests/test_models.py

@@ -149,7 +149,8 @@ def test_other_geometry_types(self):
 
 class TestBlacklistedUserModel(TestCase):
     def setUp(self):
-        self.user = UserFactory(is_staff=True)
+        self.user = UserFactory()
+        self.other_user = UserFactory()
         self.blacklisted = BlacklistedUser.objects.create(
             username='Bad User',
             uid='3434',
@@ -187,4 +188,9 @@ def test_validation(self):
             uid='5643',
             added_by=self.user,
             )
-        self.assertEqual(BlacklistedUser.objects.count(), 2)
+        BlacklistedUser.objects.create(
+            username='Bad User',
+            uid='5643',
+            added_by=self.other_user,
+            )
+        self.assertEqual(BlacklistedUser.objects.count(), 3)

osmchadjango/supervise/tests/test_views.py

@@ -844,6 +844,11 @@ def setUp(self):
             uid='3435',
             added_by=self.staff_user,
             )
+        BlacklistedUser.objects.create(
+            username='New bad user',
+            uid='9888',
+            added_by=self.user,
+            )
         self.url = reverse('supervise:blacklist-list-create')
 
     def test_list_view_unauthenticated(self):
@@ -853,7 +858,8 @@ def test_list_view_unauthenticated(self):
     def test_list_view_normal_user(self):
         self.client.login(username=self.user.username, password='password')
         response = self.client.get(self.url)
-        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(response.data.get('results')), 1)
 
     def test_list_view_staff_user(self):
         self.client.login(username=self.staff_user.username, password='password')
@@ -896,8 +902,8 @@ def test_create_view_unauthenticated(self):
     def test_create_view_normal_user(self):
         self.client.login(username=self.user.username, password='password')
         response = self.client.post(self.url, self.data)
-        self.assertEqual(response.status_code, 403)
-        self.assertEqual(BlacklistedUser.objects.count(), 0)
+        self.assertEqual(response.status_code, 201)
+        self.assertEqual(BlacklistedUser.objects.count(), 1)
 
     def test_create_view_staff_user(self):
         self.client.login(username=self.staff_user.username, password='password')
@@ -935,6 +941,11 @@ def setUp(self):
             uid='3434',
             added_by=self.staff_user,
             )
+        self.blacklisted_2 = BlacklistedUser.objects.create(
+            username='Bad User',
+            uid='3434',
+            added_by=self.user,
+            )
         self.url = reverse(
             'supervise:blacklist-detail', args=[self.blacklisted.uid]
             )
@@ -946,7 +957,23 @@ def test_unauthenticated_get(self):
     def test_normal_user_get(self):
         self.client.login(username=self.user.username, password='password')
         response = self.client.get(self.url)
-        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.data.get('username'), 'Bad User')
+        self.assertEqual(response.data.get('added_by'), 'test_user')
+        self.assertIsNotNone(response.data.get('uid'))
+        self.assertIn('date', response.data.keys())
+
+    def test_normal_user_getting_staff_user_blacklist(self):
+        blacklisted = BlacklistedUser.objects.create(
+            username='Bad User',
+            uid='4999',
+            added_by=self.staff_user,
+            )
+        self.client.login(username=self.user.username, password='password')
+        response = self.client.get(
+            reverse('supervise:blacklist-detail', args=[4999])
+            )
+        self.assertEqual(response.status_code, 404)
 
     def test_staff_user_get(self):
         self.client.login(username=self.staff_user.username, password='password')
@@ -960,19 +987,19 @@ def test_staff_user_get(self):
     def test_unauthenticated_delete(self):
         response = self.client.delete(self.url)
         self.assertEqual(response.status_code, 401)
-        self.assertEqual(BlacklistedUser.objects.count(), 1)
+        self.assertEqual(BlacklistedUser.objects.count(), 2)
 
     def test_normal_user_delete(self):
         self.client.login(username=self.user.username, password='password')
         response = self.client.delete(self.url)
-        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.status_code, 204)
         self.assertEqual(BlacklistedUser.objects.count(), 1)
 
     def test_staff_user_delete(self):
         self.client.login(username=self.staff_user.username, password='password')
         response = self.client.delete(self.url)
         self.assertEqual(response.status_code, 204)
-        self.assertEqual(BlacklistedUser.objects.count(), 0)
+        self.assertEqual(BlacklistedUser.objects.count(), 1)
 
     def test_unauthenticated_patch(self):
         response = self.client.patch(self.url, {'username': 'other_user'})
@@ -982,8 +1009,9 @@ def test_unauthenticated_patch(self):
     def test_normal_user_patch(self):
         self.client.login(username=self.user.username, password='password')
         response = self.client.patch(self.url, {'username': 'other_user'})
-        self.assertEqual(response.status_code, 403)
-        self.assertEqual(self.blacklisted.username, 'Bad User')
+        self.assertEqual(response.status_code, 200)
+        self.blacklisted_2.refresh_from_db()
+        self.assertEqual(self.blacklisted_2.username, 'other_user')
 
     def test_staff_user_patch(self):
         self.client.login(username=self.staff_user.username, password='password')

osmchadjango/supervise/views.py

@@ -5,7 +5,7 @@
 from django.urls import reverse
 
 from rest_framework.generics import (
-    ListCreateAPIView, ListAPIView, RetrieveUpdateDestroyAPIView
+    ListCreateAPIView, ListAPIView, RetrieveUpdateDestroyAPIView, get_object_or_404
     )
 from rest_framework.response import Response
 from rest_framework.filters import OrderingFilter
@@ -222,7 +222,13 @@ class BlacklistedUserListCreateAPIView(ListCreateAPIView):
     """
     queryset = BlacklistedUser.objects.all()
     serializer_class = BlacklistSerializer
-    permission_classes = (IsAdminUser,)
+    permission_classes = (IsAuthenticated,)
+
+    def get_queryset(self):
+        if self.request:
+            return BlacklistedUser.objects.filter(added_by=self.request.user)
+        else:
+            BlacklistedUser.objects.none()
 
     def perform_create(self, serializer):
         serializer.save(added_by=self.request.user)
@@ -234,21 +240,25 @@ class BlacklistedUserDetailAPIView(RetrieveUpdateDestroyAPIView):
     Get details about a BlacklistedUser.
     Access restricted to staff users.
     delete:
-    Delete a User from the Blacklist.
-    Only staff users can use this method.
+    Delete a User from your Blacklist.
     patch:
     Update a BlacklistedUser.
-    It's useful if you need to update the username of a User. Only staff users
-    can use this method.
+    It's useful if you need to update the username of a User.
     put:
     Update a BlacklistedUser.
-    It's useful if you need to update the username of a User. Only staff users
-    can use this method.
+    It's useful if you need to update the username of a User.
     """
     queryset = BlacklistedUser.objects.all()
     serializer_class = BlacklistSerializer
-    permission_classes = (IsAdminUser,)
-    lookup_field = 'uid'
+    permission_classes = (IsAuthenticated, IsOwnerOrReadOnly)
 
     def perform_update(self, serializer):
         serializer.save(added_by=self.request.user)
+
+    def get_object(self):
+        queryset = self.get_queryset()
+        return get_object_or_404(
+            queryset,
+            added_by=self.request.user,
+            uid=self.kwargs['uid']
+            )

requirements/test.txt

@@ -1,6 +1,5 @@
 # Test dependencies go here.
 -r base.txt
-flake8==3.5.0
-django-test-plus==1.0.22
-factory-boy==2.9.2
-mock==2.0.0
+flake8==3.6.0
+django-test-plus==1.1.1
+factory-boy==2.11.1