From fdbea6fb5e7ea33b9fe31c48ffcf65805e4a524c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Mon, 11 May 2020 15:34:40 +0200
Subject: [PATCH] import: do not fail if page.parent is not found (#22889)

---
 combo/data/models.py        | 25 ++++++++++++----------
 combo/data/utils.py         |  4 ++--
 combo/manager/views.py      |  3 ++-
 tests/test_import_export.py | 41 +++++++++++++++++++++++++++++++------
 tests/test_manager.py       | 20 ++++++++++++++++++
 5 files changed, 73 insertions(+), 20 deletions(-)

diff --git a/combo/data/models.py b/combo/data/models.py
index 1e4b14c..c7fa07f 100644
--- a/combo/data/models.py
+++ b/combo/data/models.py
@@ -29,6 +29,7 @@ import subprocess
 
 from django.apps import apps
 from django.conf import settings
+from django.contrib import messages
 from django.contrib.auth.models import Group
 from django.contrib.contenttypes.fields import GenericForeignKey
 from django.contrib.contenttypes.fields import GenericRelation
@@ -399,11 +400,21 @@ class Page(models.Model):
         return serialized_page
 
     @classmethod
-    def load_serialized_page(cls, json_page, snapshot=None):
+    def load_serialized_page(cls, json_page, snapshot=None, request=None):
         json_page['model'] = 'data.page'
         json_page['fields']['groups'] = [[x] for x in json_page['fields']['groups'] if isinstance(x, six.string_types)]
         page, created = Page.objects.get_or_create(slug=json_page['fields']['slug'], snapshot=snapshot)
         json_page['pk'] = page.id
+        parent_slug = json_page['fields'].get('parent') or []
+        if parent_slug and not Page.objects.filter(slug=parent_slug[0]).exists():
+            # parent not found, remove it and exclude page from navigation
+            json_page['fields'].pop('parent')
+            json_page['fields']['exclude_from_navigation'] = True
+            if request:
+                messages.warning(
+                    request,
+                    _('Unknown parent for page "%s"; parent has been reset and page was excluded from navigation.')
+                    % json_page['fields']['title'])
         page = next(serializers.deserialize('json', json.dumps([json_page]), ignorenonexistent=True))
         page.object.snapshot = snapshot
         page.save()
@@ -414,7 +425,6 @@ class Page(models.Model):
             else:
                 cell['fields']['page'] = page.object.natural_key()
 
-
         # if there were cells, remove them
         for cell in CellBase.get_cells(page_id=page.object.id):
             cell.delete()
@@ -433,20 +443,13 @@ class Page(models.Model):
             cell.object.import_subobjects(cell_data)
 
     @classmethod
-    def load_serialized_pages(cls, json_site):
+    def load_serialized_pages(cls, json_site, request=None):
         cells = []
         for json_page in json_site:
-            cls.load_serialized_page(json_page)
+            cls.load_serialized_page(json_page, request=request)
             cells.extend(json_page.get('cells'))
         cls.load_serialized_cells(cells)
 
-        # 2nd pass to set parents
-        for json_page in json_site:
-            if json_page.get('parent_slug'):
-                page = Page.objects.get(slug=json_page['fields']['slug'])
-                page.parent = Page.objects.get(slug=json_page.get('parent_slug'))
-                page.save()
-
     @classmethod
     def export_all_for_json(cls):
         ordered_pages = Page.get_as_reordered_flat_hierarchy(cls.objects.all())
diff --git a/combo/data/utils.py b/combo/data/utils.py
index adf5ed9..93365ff 100644
--- a/combo/data/utils.py
+++ b/combo/data/utils.py
@@ -47,7 +47,7 @@ def export_site():
            }
 
 
-def import_site(data, if_empty=False, clean=False):
+def import_site(data, if_empty=False, clean=False, request=None):
     if isinstance(data, list):
         # old export form with a list of pages, convert it to new dictionary
         # format.
@@ -80,7 +80,7 @@ def import_site(data, if_empty=False, clean=False):
 
         MapLayer.load_serialized_objects(data.get('map-layers') or [])
         Asset.load_serialized_objects(data.get('assets') or [])
-        Page.load_serialized_pages(data.get('pages') or [])
+        Page.load_serialized_pages(data.get('pages') or [], request=request)
 
         if data.get('pwa'):
             PwaSettings.load_serialized_settings(data['pwa'].get('settings'))
diff --git a/combo/manager/views.py b/combo/manager/views.py
index e62099f..68d9ffe 100644
--- a/combo/manager/views.py
+++ b/combo/manager/views.py
@@ -85,13 +85,14 @@ class SiteImportView(FormView):
             return self.form_invalid(form)
 
         try:
-            import_site(json_site)
+            import_site(json_site, request=self.request)
         except MissingGroups as e:
             form.add_error('site_json', force_text(e))
             return self.form_invalid(form)
 
         return super(SiteImportView, self).form_valid(form)
 
+
 site_import = SiteImportView.as_view()
 
 
diff --git a/tests/test_import_export.py b/tests/test_import_export.py
index 3e55dee..e8ab90c 100644
--- a/tests/test_import_export.py
+++ b/tests/test_import_export.py
@@ -26,15 +26,13 @@ pytestmark = pytest.mark.django_db
 
 @pytest.fixture
 def some_data():
-    page = Page(title='One', slug='one')
-    page.save()
-    page = Page(title='Two', slug='two')
-    page.save()
-    page = Page(title='Three', slug='three')
-    page.save()
+    Page.objects.create(title='One', slug='one')
+    Page.objects.create(title='Two', slug='two')
+    page = Page.objects.create(title='Three', slug='three')
     cell = TextCell(page=page, order=0, text='hello world', placeholder='content')
     cell.save()
 
+
 @pytest.fixture
 def some_map_layers():
     MapLayer(label='Foo', slug='foo', geojson_url='http://example.net/foo/').save()
@@ -52,6 +50,7 @@ def get_output_of_command(command, *args, **kwargs):
     sys.stdout = old_stdout
     return output.getvalue()
 
+
 def test_import_export(app, some_data):
     output = get_output_of_command('export_site')
     assert len(json.loads(output)['pages']) == 3
@@ -89,6 +88,36 @@ def test_import_export(app, some_data):
     assert os.path.exists(os.path.join(tempdir, 't.json'))
     shutil.rmtree(tempdir)
 
+
+def test_import_export_with_parent(app, some_data):
+    output = get_output_of_command('export_site')
+    payload = json.loads(output)
+    payload['pages'][1]['fields']['parent'] = ['one']
+
+    Page.objects.all().delete()
+    import_site(data=payload)
+
+    assert Page.objects.count() == 3
+    two = Page.objects.get(slug='two')
+    assert two.parent.slug == 'one'
+
+
+def test_import_export_with_unknown_parent(app, some_data):
+    output = get_output_of_command('export_site')
+    payload = json.loads(output)
+    payload['pages'][0]['fields']['exclude_from_navigation'] = False
+    payload['pages'][0]['fields']['parent'] = ['unknown-parent']
+
+    Page.objects.all().delete()
+    import_site(data=payload)
+
+    assert Page.objects.count() == 3
+    for page in Page.objects.all():
+        assert page.parent is None
+    one = Page.objects.get(slug='one')
+    assert one.exclude_from_navigation is True
+
+
 def test_backward_compatibility_import(app, some_data):
     old_export = Page.export_all_for_json()
     Page.objects.all().delete()
diff --git a/tests/test_manager.py b/tests/test_manager.py
index c41f3dd..d4789a1 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -15,6 +15,7 @@ from django.template import TemplateSyntaxError
 from django.test import override_settings
 from django.test.client import RequestFactory
 from django.test.utils import CaptureQueriesContext
+from django.utils.encoding import force_bytes
 from django.utils.http import urlencode
 from django.utils.six import BytesIO
 from django.utils.six.moves.urllib import parse as urlparse
@@ -585,6 +586,7 @@ def test_site_export_import(app, admin_user):
     resp = resp.form.submit()
     assert 'File is not in the expected JSON format.' in resp.text
 
+
 def test_site_export_import_missing_group(app, admin_user):
     Page.objects.all().delete()
     group = Group.objects.create(name='foobar')
@@ -609,6 +611,24 @@ def test_site_export_import_missing_group(app, admin_user):
     assert 'Missing groups: foobar' in resp.text
 
 
+def test_site_export_import_unknown_parent(app, admin_user):
+    Page.objects.create(title='One', slug='one', template_name='standard')
+    Page.objects.create(title='Two', slug='two', template_name='standard')
+
+    app = login(app)
+    resp = app.get('/manage/')
+    resp = resp.click('Export Site')
+    payload = json.loads(resp.body)
+    payload['pages'][0]['fields']['exclude_from_navigation'] = False
+    payload['pages'][0]['fields']['parent'] = ['unknown-parent']
+
+    resp = app.get('/manage/')
+    resp = resp.click('Import Site')
+    resp.form['site_json'] = Upload('site-export.json', force_bytes(json.dumps(payload)), 'application/json')
+    resp = resp.form.submit().follow()
+    assert 'Unknown parent for page &quot;One&quot;; parent has been reset and page was excluded from navigation.' in resp.text
+
+
 def test_invalid_cell_report(app, admin_user):
     app = login(app)
     resp = app.get('/manage/cells/invalid-report/')
-- 
2.20.1

