remove calls to Feature Models in tests and views

osmchadjango/changeset/migrations/0053_auto_20180815_2101.py

@@ -0,0 +1,19 @@
+# Generated by Django 2.0.6 on 2018-08-15 21:01
+
+import django.contrib.postgres.fields.jsonb
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('changeset', '0052_auto_20180809_1210'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='changeset',
+            name='new_features',
+            field=django.contrib.postgres.fields.jsonb.JSONField(default=list),
+        ),
+    ]

osmchadjango/changeset/models.py

@@ -68,7 +68,7 @@ class Changeset(models.Model):
     imagery_used = models.CharField(max_length=1000, blank=True, null=True, db_index=True)
     date = models.DateTimeField(null=True, db_index=True)
     reasons = models.ManyToManyField(SuspicionReasons, related_name='changesets')
-    new_features = JSONField(blank=True, null=True)
+    new_features = JSONField(default=list)
     create = models.IntegerField(db_index=True, null=True)
     modify = models.IntegerField(db_index=True, null=True)
     delete = models.IntegerField(db_index=True, null=True)

osmchadjango/changeset/serializers.py

@@ -35,50 +35,19 @@ class Meta:
         fields = ('id', 'name')
 
 
-class FeatureSimpleSerializerToStaff(ModelSerializer):
-    name = SerializerMethodField()
-    reasons = BasicSuspicionReasonsSerializer(many=True, read_only=True)
-
-    class Meta:
-        model = Feature
-        fields = ('osm_id', 'url', 'name', 'reasons')
-
-    def get_name(self, obj):
-        try:
-            return obj.geojson['properties']['name']
-        except TypeError:
-            try:
-                return json.loads(obj.geojson)['properties']['name']
-            except KeyError:
-                return None
-        except KeyError:
-            return None
-
-
-class FeatureSimpleSerializer(FeatureSimpleSerializerToStaff):
-    reasons = SerializerMethodField()
-
-    def get_reasons(self, obj):
-        return BasicSuspicionReasonsSerializer(
-            obj.reasons.filter(is_visible=True),
-            many=True,
-            read_only=True
-            ).data
-
-
 class ChangesetSerializerToStaff(GeoFeatureModelSerializer):
     """Serializer with all the Changeset model fields, except the
     'powerfull_editor'.
     """
     check_user = ReadOnlyField(source='check_user.name', default=None)
     reasons = BasicSuspicionReasonsSerializer(many=True, read_only=True)
     tags = BasicTagSerializer(many=True, read_only=True)
-    features = FeatureSimpleSerializerToStaff(many=True, read_only=True)
+    features = ReadOnlyField(source='new_features', default=list)
 
     class Meta:
         model = Changeset
         geo_field = 'bbox'
-        exclude = ('powerfull_editor',)
+        exclude = ('powerfull_editor', 'new_features')
 
 
 class ChangesetSerializer(ChangesetSerializerToStaff):
@@ -88,17 +57,6 @@ class ChangesetSerializer(ChangesetSerializerToStaff):
     """
     reasons = SerializerMethodField()
     tags = SerializerMethodField()
-    features = SerializerMethodField()
-
-    def get_features(self, obj):
-        """Filter features to show only the ones that have at least one visible
-        Suspicion Reason.
-        """
-        return FeatureSimpleSerializer(
-            obj.features.filter(reasons__in=obj.reasons.filter(is_visible=True)),
-            many=True,
-            read_only=True
-            ).data
 
     def get_reasons(self, obj):
         return BasicSuspicionReasonsSerializer(
@@ -224,18 +182,6 @@ class Meta:
         fields = ('changesets',)
 
 
-class SuspicionReasonsFeatureSerializer(ModelSerializer):
-    features = PrimaryKeyRelatedField(
-        many=True,
-        queryset=Feature.objects.all(),
-        help_text='List of features ids.'
-        )
-
-    class Meta:
-        model = SuspicionReasons
-        fields = ('features',)
-
-
 class ChangesetTagsSerializer(ModelSerializer):
     tags = PrimaryKeyRelatedField(
         many=True,

osmchadjango/changeset/tests/test_changeset_views.py

@@ -8,7 +8,6 @@
 from rest_framework.test import APITestCase
 
 from ...users.models import User
-from ...feature.tests.modelfactories import FeatureFactory
 from ..models import SuspicionReasons, Tag, Changeset
 from ..views import ChangesetListAPIView, PaginatedCSVRenderer
 from .modelfactories import (
@@ -364,19 +363,22 @@ class TestChangesetDetailView(APITestCase):
 
     def setUp(self):
         self.reason_1 = SuspicionReasons.objects.create(name='possible import')
-        self.reason_2 = SuspicionReasons.objects.create(name='suspect word')
-        self.reason_3 = SuspicionReasons.objects.create(
+        self.reason_2 = SuspicionReasons.objects.create(
             name='Big edit in my city',
             is_visible=False
             )
-        self.changeset = HarmfulChangesetFactory(id=31982803)
-        self.feature = FeatureFactory(changeset=self.changeset)
-        self.invisible_feature = FeatureFactory(changeset=self.changeset)
+        self.changeset = HarmfulChangesetFactory(
+            id=31982803,
+            new_features=[{
+                "osm_id": 87765444,
+                "url": "node-87765444",
+                "version": 44,
+                "reasons": [self.reason_1.id, self.reason_2.id],
+                "name": "Test"
+            }]
+            )
         self.reason_1.changesets.add(self.changeset)
         self.reason_2.changesets.add(self.changeset)
-        self.reason_2.features.add(self.feature)
-        self.reason_3.features.add(self.feature, self.invisible_feature)
-        self.reason_3.changesets.add(self.changeset)
         self.tag = Tag.objects.create(name='Vandalism')
         self.tag.changesets.add(self.changeset)
 
@@ -424,25 +426,32 @@ def test_unauthenticated_changeset_detail_response(self):
         self.assertTrue(response.data['properties']['harmful'])
         self.assertIn('date', response.data['properties'].keys())
         self.assertIn('check_date', response.data['properties'].keys())
+        # unauthenticated users will have access to only the visible reason
+        self.assertEqual(len(response.data['properties']['reasons']), 1)
         self.assertEqual(len(response.data['properties']['features']), 1)
         self.assertEqual(
-            self.feature.osm_id,
+            87765444,
             response.data['properties']['features'][0]['osm_id']
             )
         self.assertEqual(
-            self.feature.url,
+            'node-87765444',
             response.data['properties']['features'][0]['url']
             )
         self.assertEqual(
             response.data['properties']['features'][0]['name'],
             'Test'
             )
+        # the non visible reason id will be serialized in the features
         self.assertEqual(
             len(response.data['properties']['features'][0]['reasons']),
-            1
+            2
             )
         self.assertIn(
-            {'id': self.reason_2.id, 'name': 'suspect word'},
+            self.reason_1.id,
+            response.data['properties']['features'][0]['reasons']
+            )
+        self.assertIn(
+            self.reason_2.id,
             response.data['properties']['features'][0]['reasons']
             )
 
@@ -490,28 +499,13 @@ def test_changeset_detail_response_with_staff_user(self):
         self.assertEqual(response.status_code, 200)
         self.assertEqual(
             len(response.data['properties']['features']),
-            2
-            )
-        self.assertIn(
-            {'id': self.reason_2.id, 'name': 'suspect word'},
-            response.data['properties']['features'][0]['reasons']
+            1
             )
         self.assertIn(
-            {'id': self.reason_3.id, 'name': 'Big edit in my city'},
+            self.reason_2.id,
             response.data['properties']['features'][0]['reasons']
             )
 
-    def test_feature_without_name_tag(self):
-        self.feature.geojson = json.dumps({'properties': {'osm:type': 'node'}})
-        self.feature.save()
-        response = self.client.get(
-            reverse('changeset:detail', args=[self.changeset.id])
-            )
-        self.assertEqual(response.status_code, 200)
-        self.assertIsNone(
-            response.data['properties']['features'][0]['name']
-            )
-
 
 class TestReasonsAndTagFieldsInChangesetViews(APITestCase):
     def setUp(self):

osmchadjango/changeset/tests/test_management_commands.py

@@ -6,11 +6,7 @@
 from django.utils.six import StringIO
 from django.utils import timezone
 
-from ...feature.models import Feature
 from ..models import Changeset, SuspicionReasons
-from ...feature.tests.modelfactories import (
-    FeatureFactory, CheckedFeatureFactory
-    )
 from .modelfactories import (
     ChangesetFactory, GoodChangesetFactory, SuspectChangesetFactory
     )
@@ -37,48 +33,28 @@ def setUp(self):
         self.six_months_ago = timezone.now() - timedelta(days=180)
         ChangesetFactory.create_batch(10, date=self.six_months_ago)
 
-        # a changeset and a feature that should be deleted
+        # a changeset that should be deleted
         self.old_changeset_1 = ChangesetFactory(date=self.six_months_ago)
-        self.feature = FeatureFactory(changeset=self.old_changeset_1)
-
-        # a feature that shouldn't be deleted, so the changeset shouldn't too
-        self.old_changeset_2 = ChangesetFactory(date=self.six_months_ago)
-        self.checked_feature = CheckedFeatureFactory(changeset=self.old_changeset_2)
-
-        # a changeset that shouldn't be deleted, so the feature shouldn't too
+        # a changeset that shouldn't be deleted
         self.old_checked_changeset = GoodChangesetFactory(date=self.six_months_ago)
-        self.old_feature_of_old_checked_changeset = FeatureFactory(
-            changeset=self.old_checked_changeset
-            )
         # two changesets that shouldn't be deleted
         self.changeset = ChangesetFactory()
         self.checked_changeset = GoodChangesetFactory()
         call_command('delete_old_data')
 
     def test_command(self):
-        self.assertEqual(Changeset.objects.count(), 4)
-        self.assertEqual(Feature.objects.count(), 2)
-        self.assertEqual(
-            Feature.objects.filter(
-                changeset__date__lt=(timezone.now() - timedelta(days=180)),
-                changeset__checked=False,
-                checked=False
-                ).count(),
-            0
-            )
+        self.assertEqual(Changeset.objects.count(), 3)
         self.assertEqual(
             Changeset.objects.filter(
                 date__lt=(timezone.now() - timedelta(days=180)),
                 checked=False,
-                features=None
                 ).count(),
             0
             )
 
         self.assertIn(self.changeset, Changeset.objects.all())
         self.assertIn(self.old_checked_changeset, Changeset.objects.all())
         self.assertIn(self.checked_changeset, Changeset.objects.all())
-        self.assertIn(self.old_changeset_2, Changeset.objects.all())
 
 
 class TestMergeReasons(TestCase):

osmchadjango/changeset/tests/test_models.py

@@ -130,10 +130,14 @@ def test_tag(self):
         tag_2.changesets.add(self.changeset)
         self.assertEqual(self.changeset.tags.all().count(), 2)
 
+    def test_empty_new_feature_field(self):
+        self.assertEqual(self.changeset.new_features, [])
+
+
     def test_new_feature_field(self):
         json_content = [
-            {"id": 123, "reasons": ["Deleted wikidata object", "Deleted place"]},
-            {"id": 321, "reasons": ["Deleted address"]}
+            {"osm_id": 123, "url": "node-123", "reasons": [1, 2], },
+            {"osm_id": 321, "url": "way-321", "reasons": [13], 'note': 'Test'}
         ]
         changeset = ChangesetFactory(
             id=31982804,