Allow to control version fields

Author: yohanboniface

ban/core/models.py

@@ -23,8 +23,8 @@ class BaseModel(BaseResource, BaseVersioned):
 class Model(ResourceModel, Versioned, metaclass=BaseModel):
     resource_fields = ['version', 'created_at', 'created_by', 'modified_at',
                        'modified_by', 'attributes']
-    exclude_for_compact = ['version', 'created_at', 'created_by',
-                           'modified_at', 'modified_by']
+    exclude_for_collection = ['version', 'created_at', 'created_by',
+                              'modified_at', 'modified_by']
     # 'version' is validated by us.
     resource_schema = {'version': {'required': False},
                        'created_at': {'readonly': True},
@@ -58,6 +58,7 @@ def __str__(self):
 class Municipality(NamedModel):
     identifiers = ['siren', 'insee']
     resource_fields = ['name', 'alias', 'insee', 'siren', 'postcodes']
+    exclude_for_version = ['postcodes']
 
     insee = db.CharField(max_length=5, unique=True)
     siren = db.CharField(max_length=9, unique=True, null=True)

ban/core/resource.py

@@ -34,8 +34,8 @@ def as_resource_list(self):
 
 class BaseResource(peewee.BaseModel):
 
-    def include_field_for_compact(cls, name):
-        if name in cls.exclude_for_compact:
+    def include_field_for_collection(cls, name):
+        if name in cls.exclude_for_collection:
             return False
         attr = getattr(cls, name, None)
         exclude = (db.ManyToManyField, peewee.ReverseRelationDescriptor,
@@ -48,7 +48,8 @@ def __new__(mcs, name, bases, attrs, **kwargs):
         # Inherit and extend instead of replacing.
         resource_fields = attrs.pop('resource_fields', None)
         resource_schema = attrs.pop('resource_schema', None)
-        exclude_for_compact = attrs.pop('exclude_for_compact', None)
+        exclude_for_collection = attrs.pop('exclude_for_collection', None)
+        exclude_for_version = attrs.pop('exclude_for_version', None)
         cls = super().__new__(mcs, name, bases, attrs, **kwargs)
         if resource_fields is not None:
             inherited = getattr(cls, 'resource_fields', {})
@@ -58,14 +59,20 @@ def __new__(mcs, name, bases, attrs, **kwargs):
             inherited = getattr(cls, 'resource_schema', {})
             resource_schema.update(inherited)
             cls.resource_schema = resource_schema
-        if exclude_for_compact is not None:
-            inherited = getattr(cls, 'exclude_for_compact', [])
-            exclude_for_compact.extend(inherited)
-            cls.exclude_for_compact = exclude_for_compact
-        cls.extended_fields = cls.resource_fields
-        cls.compact_fields = [
-            n for n in cls.extended_fields
-            if mcs.include_field_for_compact(cls, n)] + ['resource']
+        if exclude_for_collection is not None:
+            inherited = getattr(cls, 'exclude_for_collection', [])
+            exclude_for_collection.extend(inherited)
+            cls.exclude_for_collection = exclude_for_collection
+        if exclude_for_version is not None:
+            inherited = getattr(cls, 'exclude_for_version', [])
+            exclude_for_version.extend(inherited)
+            cls.exclude_for_version = exclude_for_version
+        cls.collection_fields = [
+            n for n in cls.resource_fields
+            if mcs.include_field_for_collection(cls, n)] + ['resource']
+        cls.versioned_fields = [
+            n for n in cls.resource_fields
+            if n not in cls.exclude_for_version]
         cls.build_resource_schema()
         return cls
 
@@ -74,7 +81,8 @@ class ResourceModel(db.Model, metaclass=BaseResource):
     resource_fields = ['id']
     identifiers = []
     resource_schema = {'id': {'readonly': True}}
-    exclude_for_compact = []
+    exclude_for_collection = []
+    exclude_for_version = []
 
     id = db.CharField(max_length=50, unique=True, null=False)
 
@@ -96,7 +104,7 @@ def build_resource_schema(cls):
         """Map Peewee models to Cerberus validation schema."""
         schema = dict(cls.resource_schema)
         for name, field in cls._meta.fields.items():
-            if name not in cls.extended_fields:
+            if name not in cls.resource_fields:
                 continue
             if field.primary_key:
                 continue
@@ -138,17 +146,17 @@ def resource(self):
     @property
     def as_resource(self):
         """Resource plus relations."""
-        return {f: self.extended_field(f) for f in self.extended_fields}
+        return {f: self.extended_field(f) for f in self.resource_fields}
 
     @property
     def as_relation(self):
         """Resources plus relation references without metadata."""
-        return {f: self.compact_field(f) for f in self.compact_fields}
+        return {f: self.compact_field(f) for f in self.collection_fields}
 
     @property
     def as_version(self):
         """Resources plus relations references and metadata."""
-        return {f: self.compact_field(f) for f in self.extended_fields}
+        return {f: self.compact_field(f) for f in self.versioned_fields}
 
     def extended_field(self, name):
         value = getattr(self, '{}_extended'.format(name), getattr(self, name))

ban/tests/test_diff.py

@@ -0,0 +1,36 @@
+from .factories import MunicipalityFactory
+
+
+def test_only_changed_columns_should_be_in_diff():
+    municipality = MunicipalityFactory(name='Moret-sur-Loing', insee='77316',
+                                       siren='987654321')
+    municipality.name = 'Orvanne'
+    municipality.siren = '123456789'
+    # "Changed" with same value.
+    municipality.insee = '77316'
+    municipality.increment_version()
+    municipality.save()
+    diff = municipality.versions[1].diff
+    assert len(diff.diff) == 2  # name, siren
+    assert diff.diff['name']['old'] == 'Moret-sur-Loing'
+    assert diff.diff['name']['new'] == 'Orvanne'
+    assert diff.diff['siren']['old'] == '987654321'
+    assert diff.diff['siren']['new'] == '123456789'
+    municipality.insee = '77319'
+    municipality.increment_version()
+    municipality.save()
+    diff = municipality.versions[2].diff
+    assert len(diff.diff) == 1  # insee
+    assert diff.diff['insee']['old'] == '77316'
+    assert diff.diff['insee']['new'] == '77319'
+
+
+def test_adding_value_to_arrayfield_column():
+    municipality = MunicipalityFactory(name='Moret-sur-Loing')
+    municipality.alias = ['Orvanne']
+    municipality.increment_version()
+    municipality.save()
+    diff = municipality.versions[1].diff
+    assert len(diff.diff) == 1  # name, siren
+    assert diff.diff['alias']['old'] is None
+    assert diff.diff['alias']['new'] == ['Orvanne']

ban/tests/test_managers.py

@@ -1,6 +1,6 @@
 from ban.core import models
 
-from .factories import MunicipalityFactory, GroupFactory
+from .factories import GroupFactory, MunicipalityFactory
 
 
 def test_municipality_as_resource():