Merge pull request #314 from willemarcel/fix/500-error-on-aoi-feed

fix error on Aoi RSS Feed view and improve aoi query logic

osmchadjango/changeset/filters.py

@@ -240,7 +240,7 @@ class ChangesetFilter(GeoFilterSet):
         )
 
     def filter_whitelist(self, queryset, name, value):
-        if (self.request is None or self.request.user.is_authenticated) and value:
+        if value:
             whitelist = self.request.user.whitelists.values_list(
                 'whitelist_user',
                 flat=True
@@ -250,7 +250,7 @@ def filter_whitelist(self, queryset, name, value):
             return queryset
 
     def filter_blacklist(self, queryset, name, value):
-        if (self.request is None or self.request.user.is_authenticated) and value:
+        if value:
             blacklist = self.request.user.blacklisteduser_set.values_list(
                 'uid',
                 flat=True

osmchadjango/supervise/models.py

@@ -2,6 +2,7 @@
 
 from django.contrib.gis.db import models
 from django.contrib.postgres.fields import JSONField
+from django.http.request import HttpRequest
 
 from osmchadjango.changeset.filters import ChangesetFilter
 from osmchadjango.feature.filters import FeatureFilter
@@ -22,8 +23,11 @@ def __str__(self):
 
     def changesets(self, request=None):
         """Return the changesets that match the filters, including the geometry
-        of the AreaOfInterest.
+        of the AreaOfInterest. Fake a request object in order to execute the
+        query with the user that created the AoI, not with the request user.
         """
+        request = HttpRequest
+        request.user = self.user
         qs = ChangesetFilter(self.filters, request=request).qs
         if self.geometry is not None:
             return qs.filter(

osmchadjango/supervise/tests/test_views.py

@@ -647,7 +647,19 @@ def test_aoi_with_hide_whitelist_filter(self):
                 'hide_whitelist': 'True'
                 },
             )
+        user_2 = User.objects.create_user(
+            username='user_2',
+            email='b@a.com',
+            password='password'
+            )
+        UserSocialAuth.objects.create(
+            user=user_2,
+            provider='openstreetmap',
+            uid='42344',
+            )
         UserWhitelistFactory(user=self.user, whitelist_user='test')
+        UserWhitelistFactory(user=user_2, whitelist_user='other_user')
+        UserWhitelistFactory(user=user_2, whitelist_user='another_user')
         ChangesetFactory()
         ChangesetFactory(user='other_user', uid='333')
         ChangesetFactory(user='another_user', uid='4333')
@@ -659,6 +671,17 @@ def test_aoi_with_hide_whitelist_filter(self):
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data['count'], 2)
         self.assertEqual(len(response.data['features']), 2)
+        self.client.logout()
+
+        # test with a second user to assure the results of the hide_whitelist filter
+        # are the same, it doesn't matter the user that is accessing
+        self.client.login(username=self.user.username, password='password')
+        response = self.client.get(
+            reverse('supervise:aoi-list-changesets', args=[aoi.pk])
+            )
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.data['count'], 2)
+        self.assertEqual(len(response.data['features']), 2)
 
     def test_aoi_with_false_hide_whitelist_filter(self):
         aoi = AreaOfInterest.objects.create(
@@ -669,7 +692,19 @@ def test_aoi_with_false_hide_whitelist_filter(self):
                 'hide_whitelist': 'False'
                 },
             )
+        user_2 = User.objects.create_user(
+            username='user_2',
+            email='b@a.com',
+            password='password'
+            )
+        UserSocialAuth.objects.create(
+            user=user_2,
+            provider='openstreetmap',
+            uid='42344',
+            )
         UserWhitelistFactory(user=self.user, whitelist_user='test')
+        UserWhitelistFactory(user=user_2, whitelist_user='other_user')
+        UserWhitelistFactory(user=user_2, whitelist_user='another_user')
         ChangesetFactory()
         ChangesetFactory(user='other_user', uid='333')
         ChangesetFactory(user='another_user', uid='4333')
@@ -681,6 +716,17 @@ def test_aoi_with_false_hide_whitelist_filter(self):
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data['count'], 3)
         self.assertEqual(len(response.data['features']), 3)
+        self.client.logout()
+
+        # test with a second user to assure the results of the hide_whitelist=False
+        # filter are the same, it doesn't matter the user that is accessing
+        self.client.login(username=user_2.username, password='password')
+        response = self.client.get(
+            reverse('supervise:aoi-list-changesets', args=[aoi.pk])
+            )
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.data['count'], 3)
+        self.assertEqual(len(response.data['features']), 3)
 
     def test_aoi_with_blacklist_filter(self):
         aoi = AreaOfInterest.objects.create(
@@ -696,6 +742,26 @@ def test_aoi_with_blacklist_filter(self):
             uid='123123',
             added_by=self.user,
             )
+        user_2 = User.objects.create_user(
+            username='user_2',
+            email='b@a.com',
+            password='password'
+            )
+        UserSocialAuth.objects.create(
+            user=user_2,
+            provider='openstreetmap',
+            uid='42344',
+            )
+        BlacklistedUser.objects.create(
+            username='other_user',
+            uid='333',
+            added_by=user_2,
+            )
+        BlacklistedUser.objects.create(
+            username='another_user',
+            uid='4333',
+            added_by=user_2,
+            )
         ChangesetFactory()
         ChangesetFactory(user='other_user', uid='333')
         ChangesetFactory(user='another_user', uid='4333')
@@ -706,6 +772,17 @@ def test_aoi_with_blacklist_filter(self):
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data['count'], 1)
         self.assertEqual(len(response.data['features']), 1)
+        self.client.logout()
+
+        # test with a second user to assure the results of the blacklist filter
+        # are the same, it doesn't matter the user that is accessing
+        self.client.login(username=user_2.username, password='password')
+        response = self.client.get(
+            reverse('supervise:aoi-list-changesets', args=[aoi.pk])
+            )
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.data['count'], 1)
+        self.assertEqual(len(response.data['features']), 1)
 
     def test_aoi_with_false_blacklist_filter(self):
         aoi = AreaOfInterest.objects.create(
@@ -721,6 +798,26 @@ def test_aoi_with_false_blacklist_filter(self):
             uid='123123',
             added_by=self.user,
             )
+        user_2 = User.objects.create_user(
+            username='user_2',
+            email='b@a.com',
+            password='password'
+            )
+        UserSocialAuth.objects.create(
+            user=user_2,
+            provider='openstreetmap',
+            uid='42344',
+            )
+        BlacklistedUser.objects.create(
+            username='other_user',
+            uid='333',
+            added_by=user_2,
+            )
+        BlacklistedUser.objects.create(
+            username='another_user',
+            uid='4333',
+            added_by=user_2,
+            )
         ChangesetFactory()
         ChangesetFactory(user='other_user', uid='333')
         ChangesetFactory(user='another_user', uid='4333')
@@ -732,6 +829,15 @@ def test_aoi_with_false_blacklist_filter(self):
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data['count'], 3)
         self.assertEqual(len(response.data['features']), 3)
+        self.client.logout()
+
+        self.client.login(username=user_2.username, password='password')
+        response = self.client.get(
+            reverse('supervise:aoi-list-changesets', args=[aoi.pk])
+            )
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.data['count'], 3)
+        self.assertEqual(len(response.data['features']), 3)
 
     def test_aoi_changesets_feed_view(self):
         ChangesetFactory(bbox=Polygon(((10, 10), (10, 11), (11, 11), (10, 10))))
@@ -746,7 +852,6 @@ def test_aoi_changesets_feed_view(self):
             user='çãoéí',
             bbox=Polygon(((0, 0), (0, 0.5), (0.7, 0.5), (0, 0))),
             )
-        self.client.login(username=self.user.username, password='password')
         response = self.client.get(
             reverse('supervise:aoi-changesets-feed', args=[self.aoi.pk])
             )
@@ -781,7 +886,6 @@ def test_feed_view_of_unnamed_aoi_and_zero_changesets(self):
             }
         self.aoi.save()
 
-        self.client.login(username=self.user.username, password='password')
         response = self.client.get(
             reverse('supervise:aoi-changesets-feed', args=[self.aoi.pk])
             )
@@ -797,6 +901,74 @@ def test_feed_view_of_unnamed_aoi_and_zero_changesets(self):
             )
         self.assertEqual(len(items), 1)
 
+    def test_feed_view_of_aoi_with_blacklist_filter(self):
+        BlacklistedUser.objects.create(
+            username='test_1',
+            uid='123123',
+            added_by=self.user,
+            )
+        ChangesetFactory(
+            user="test_2",
+            editor='JOSM 1.5',
+            bbox=Polygon(((10, 10), (10, 11), (11, 11), (10, 10)))
+            )
+        HarmfulChangesetFactory(
+            user="test_1",
+            editor='JOSM 1.5',
+            bbox=Polygon(((0, 0), (0, 0.5), (0.7, 0.5), (0, 0)))
+            )
+        self.aoi.name = ''
+        self.aoi.filters = {
+            'editor': 'JOSM 1.5',
+            'blacklist': 'True',
+            }
+        self.aoi.save()
+
+        response = self.client.get(
+            reverse('supervise:aoi-changesets-feed', args=[self.aoi.pk])
+            )
+        self.assertEqual(response.status_code, 200)
+        rss_data = ET.fromstring(response.content).getchildren()[0].getchildren()
+        title = [i for i in rss_data if i.tag == 'title'][0]
+        items = [i for i in rss_data if i.tag == 'item']
+        self.assertEqual(
+            title.text,
+            'Changesets of Area of Interest Unnamed by {}'.format(
+                self.aoi.user.username
+                )
+            )
+        self.assertEqual(len(items), 1)
+
+    def test_feed_view_of_aoi_with_hide_whitelist_filter(self):
+        aoi = AreaOfInterest.objects.create(
+            name='Another place in the world',
+            user=self.user,
+            filters={
+                'editor': 'Potlatch 2',
+                'hide_whitelist': 'True'
+                },
+            )
+        UserWhitelistFactory(user=self.user, whitelist_user='test')
+        ChangesetFactory()
+        ChangesetFactory(user='other_user', uid='333')
+        ChangesetFactory(user='another_user', uid='4333')
+
+
+        response = self.client.get(
+            reverse('supervise:aoi-changesets-feed', args=[aoi.pk])
+            )
+        self.assertEqual(response.status_code, 200)
+        rss_data = ET.fromstring(response.content).getchildren()[0].getchildren()
+        title = [i for i in rss_data if i.tag == 'title'][0]
+        items = [i for i in rss_data if i.tag == 'item']
+        self.assertEqual(
+            title.text,
+            'Changesets of Area of Interest Another place in the world by {}'.format(
+                self.aoi.user.username
+                )
+            )
+        self.assertEqual(len(items), 2)
+
 
 class TestAoIStatsAPIViews(APITestCase):
     def setUp(self):

osmchadjango/supervise/views.py

@@ -187,7 +187,7 @@ def get_serializer_class(self):
             return ChangesetSerializer
 
     def list(self, request, *args, **kwargs):
-        queryset = self.get_object().changesets(request).select_related(
+        queryset = self.get_object().changesets().select_related(
             'check_user'
             ).prefetch_related('tags', 'reasons')