misc: remove position field from category snapshots (#86624)

tests/test_categories.py

@@ -7,10 +7,28 @@ import pytest
 from django.utils.encoding import force_bytes
 from quixote import cleanup
 
-from wcs.categories import CardDefCategory, Category
+from wcs.categories import (
+    BlockCategory,
+    CardDefCategory,
+    Category,
+    CommentTemplateCategory,
+    DataSourceCategory,
+    MailTemplateCategory,
+    WorkflowCategory,
+)
 
 from .utilities import clean_temporary_pub, create_temporary_pub
 
+category_classes = [
+    Category,
+    CardDefCategory,
+    BlockCategory,
+    WorkflowCategory,
+    MailTemplateCategory,
+    CommentTemplateCategory,
+    DataSourceCategory,
+]
+
 
 def setup_module(module):
     cleanup()
@@ -24,7 +42,7 @@ def teardown_module(module):
     clean_temporary_pub()
 
 
-@pytest.mark.parametrize('category_class', [Category, CardDefCategory])
+@pytest.mark.parametrize('category_class', category_classes)
 def test_store(category_class):
     category_class.wipe()
     test = category_class()
@@ -37,7 +55,7 @@ def test_store(category_class):
     assert test.description == test2.description
 
 
-@pytest.mark.parametrize('category_class', [Category, CardDefCategory])
+@pytest.mark.parametrize('category_class', category_classes)
 def test_urlname(category_class):
     category_class.wipe()
     test = category_class()
@@ -48,7 +66,7 @@ def test_urlname(category_class):
     assert test.url_name == 'test'
 
 
-@pytest.mark.parametrize('category_class', [Category, CardDefCategory])
+@pytest.mark.parametrize('category_class', category_classes)
 def test_duplicate_urlname(category_class):
     category_class.wipe()
     test = category_class()
@@ -64,7 +82,7 @@ def test_duplicate_urlname(category_class):
     assert test2.url_name == 'test-2'
 
 
-@pytest.mark.parametrize('category_class', [Category, CardDefCategory])
+@pytest.mark.parametrize('category_class', category_classes)
 def test_name_giving_a_forbidden_slug(category_class):
     category_class.wipe()
     test = category_class()
@@ -74,7 +92,7 @@ def test_name_giving_a_forbidden_slug(category_class):
     assert test.url_name == 'cat-api'
 
 
-@pytest.mark.parametrize('category_class', [Category, CardDefCategory])
+@pytest.mark.parametrize('category_class', category_classes)
 def test_sort_positions(category_class):
     category_class.wipe()
 
@@ -94,7 +112,7 @@ def test_sort_positions(category_class):
     assert categories[-1].name in ('Test 8', 'Test 9')
 
 
-@pytest.mark.parametrize('category_class', [Category, CardDefCategory])
+@pytest.mark.parametrize('category_class', category_classes)
 def test_xml_export(category_class):
     category_class.wipe()
     test = category_class()
@@ -108,7 +126,7 @@ def test_xml_export(category_class):
     assert b' id="1"' not in test.export_to_xml_string(include_id=False)
 
 
-@pytest.mark.parametrize('category_class', [Category, CardDefCategory])
+@pytest.mark.parametrize('category_class', category_classes)
 def test_xml_import(category_class):
     category_class.wipe()
     test = category_class()
@@ -190,7 +208,7 @@ def test_load_old_python2_pickle():
     assert test2.description == 'Hello world'
 
 
-@pytest.mark.parametrize('category_class', [Category, CardDefCategory])
+@pytest.mark.parametrize('category_class', category_classes)
 def test_get_by_urlname(category_class):
     category_class.wipe()
     test = category_class()
@@ -202,7 +220,7 @@ def test_get_by_urlname(category_class):
     assert test.id == test2.id
 
 
-@pytest.mark.parametrize('category_class', [Category, CardDefCategory])
+@pytest.mark.parametrize('category_class', category_classes)
 def test_has_urlname(category_class):
     category_class.wipe()
     test = category_class()
@@ -215,7 +233,7 @@ def test_has_urlname(category_class):
     assert not category_class.has_urlname('foobar')
 
 
-@pytest.mark.parametrize('category_class', [Category, CardDefCategory])
+@pytest.mark.parametrize('category_class', category_classes)
 def test_remove_self(category_class):
     category_class.wipe()
     test = category_class()

tests/test_snapshots.py

@@ -1435,25 +1435,49 @@ def test_category_snapshot_browse(pub):
 
     Category.wipe()
     category = Category(name='test')
+    category.position = 42
     category.store()
     assert pub.snapshot_class.count() == 1
     # check calling .store() without changes doesn't create snapshots
     category.store()
     assert pub.snapshot_class.count() == 1
+    category.name = 'foobar'
+    category.store()
+    assert pub.snapshot_class.count() == 2
 
     app = login(get_app(pub))
 
     resp = app.get('/backoffice/forms/categories/%s/' % category.id)
     resp = resp.click('History')
-    snapshot = pub.snapshot_class.select_object_history(category)[0]
+    snapshot = pub.snapshot_class.select_object_history(category)[1]
+    snapshot = snapshot.get_latest(
+        snapshot.object_type, snapshot.object_id, complete=True, max_timestamp=snapshot.timestamp
+    )
+    assert snapshot.patch is None
+    assert 'position' not in snapshot.serialization
     resp = resp.click(href='%s/view/' % snapshot.id)
     assert 'This category is readonly' in resp.text
+    assert 'inspect' not in resp
     assert '<p>%s</p>' % localstrftime(snapshot.timestamp) in resp.text
     with pytest.raises(IndexError):
-        resp = resp.click('Edit')
+        resp.click('Edit')
+    resp = app.get('/backoffice/forms/categories/%s/' % category.id)
+    resp = resp.click('History')
+    resp = resp.click(href='%s/restore' % snapshot.id)
+    assert resp.form['action'].value == 'as-new'
+    resp = resp.form.submit('submit')
+    assert Category.count() == 2
+    new_category = Category.get(resp.location.split('/')[-2])
+    assert new_category.position == 43
 
-    resp = app.get('/backoffice/forms/categories/%s/history/%s/view/' % (category.id, snapshot.id))
-    assert 'inspect' not in resp
+    resp = app.get('/backoffice/forms/categories/%s/' % category.id)
+    resp = resp.click('History')
+    resp = resp.click(href='%s/restore' % snapshot.id)
+    resp.form['action'].value = 'overwrite'
+    resp = resp.form.submit('submit')
+    assert Category.count() == 2
+    category.refresh_from_storage()
+    assert category.position == 42
 
 
 def test_snapshots_test_results(pub):

wcs/snapshots.py

@@ -139,6 +139,16 @@ class Snapshot:
     _instance = None
     _user = None
 
+    _category_types = [
+        'block_category',
+        'card_category',
+        'data_source_category',
+        'category',
+        'mail_template_category',
+        'comment_template_category',
+        'workflow_category',
+    ]
+
     @classmethod
     def snap(cls, instance, comment=None, label=None, store_user=True, application=None):
         obj = cls()
@@ -147,7 +157,13 @@ class Snapshot:
         obj.timestamp = now()
         if get_session() and store_user:
             obj.user_id = get_session().user
+
         tree = instance.export_to_xml(include_id=True)
+        # remove position for categories
+        if obj.object_type in cls._category_types:
+            for position in tree.findall('position'):
+                tree.remove(position)
+
         obj.serialization = ET.tostring(tree).decode('utf-8')
         obj.comment = str(comment) if comment else None
         obj.label = label
@@ -321,15 +337,23 @@ class Snapshot:
         instance = self.instance
         if as_new:
             for attr in ('id', 'url_name', 'internal_identifier', 'slug'):
-                setattr(instance, attr, None)
+                try:
+                    setattr(instance, attr, None)
+                except AttributeError:
+                    # attribute can be a property without setter
+                    pass
+            if self.object_type in self._category_types:
+                # set position
+                instance.position = max([i.position or 0 for i in self.get_object_class().select()]) + 1
             if hasattr(instance, 'disabled'):
                 instance.disabled = True
         else:
-            # keep table and max field id from current object
+            # keep table and position from current object
             current_object = self.get_object_class().get(instance.id)
-            for attr in ('table_name',):
-                if hasattr(current_object, attr):
-                    setattr(instance, attr, getattr(current_object, attr))
+            for attr in ('table_name', 'position'):
+                if attr != 'position' or self.object_type in self._category_types:
+                    if hasattr(current_object, attr):
+                        setattr(instance, attr, getattr(current_object, attr))
 
         delattr(instance, 'readonly')
         delattr(instance, 'snapshot_object')