From b588638c1b9fe13275b92c9e19e6bce790553d32 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Fri, 12 Mar 2021 16:24:45 +0100
Subject: [PATCH] snapshots: do not fail on load error (#51641)

---
 tests/test_snapshots.py                     | 42 ++++++++++++++++
 wcs/backoffice/snapshots.py                 | 56 ++++++++++++++-------
 wcs/blocks.py                               |  4 +-
 wcs/qommon/xml_storage.py                   |  4 +-
 wcs/snapshots.py                            |  5 +-
 wcs/templates/wcs/backoffice/snapshots.html |  2 +
 6 files changed, 91 insertions(+), 22 deletions(-)

diff --git a/tests/test_snapshots.py b/tests/test_snapshots.py
index dc4002bf..580c1b79 100644
--- a/tests/test_snapshots.py
+++ b/tests/test_snapshots.py
@@ -10,6 +10,7 @@ from quixote.http_request import Upload
 from wcs.blocks import BlockDef
 from wcs.carddef import CardDef
 from wcs.data_sources import NamedDataSource
+from wcs.fields import ItemField
 from wcs.formdef import FormDef
 from wcs.qommon.form import UploadedFile
 from wcs.qommon.misc import localstrftime
@@ -231,6 +232,22 @@ def test_form_snapshot_restore(pub, formdef_with_history):
     assert formdef.url_name == formdef_with_history.url_name
 
 
+def test_form_snapshot_restore_with_import_error(pub):
+    create_superuser(pub)
+    create_role()
+    app = login(get_app(pub))
+
+    formdef = FormDef()
+    formdef.name = 'testform'
+    formdef.fields = [ItemField(id='1', label='Test', type='item', data_source={'type': 'unknown'})]
+    formdef.store()
+    assert pub.snapshot_class.count() == 1
+    snapshot = pub.snapshot_class.select_object_history(formdef)[0]
+    resp = app.get('/backoffice/forms/%s/history/%s/restore' % (formdef.id, snapshot.id))
+    resp = resp.form.submit('submit')
+    assert 'Can not restore snapshot (Unknown datasources [unknown])' in resp
+
+
 def test_block_snapshot_browse(pub, blocks_feature):
     create_superuser(pub)
     create_role()
@@ -376,6 +393,31 @@ def test_form_snapshot_browse(pub, formdef_with_history):
     assert pub.custom_view_class.count() == 0  # custom views are not restore on preview
 
 
+def test_form_snapshot_browse_with_import_error(pub):
+    create_superuser(pub)
+    create_role()
+    app = login(get_app(pub))
+
+    formdef = FormDef()
+    formdef.name = 'testform'
+    formdef.fields = [ItemField(id='1', label='Test', type='item', data_source={'type': 'unknown'})]
+    formdef.store()
+    assert pub.snapshot_class.count() == 1
+    snapshot = pub.snapshot_class.select_object_history(formdef)[0]
+    # no error for missing datasource
+    resp = app.get('/backoffice/forms/%s/history/%s/view/' % (formdef.id, snapshot.id), status=200)
+
+    # other FormdefImportError
+    formdef.fields = [ItemField(id='1', label='Test', type='foobar')]
+    formdef.store()
+    assert pub.snapshot_class.count() == 2
+    snapshot = pub.snapshot_class.select_object_history(formdef)[0]
+    resp = app.get('/backoffice/forms/%s/history/%s/view/' % (formdef.id, snapshot.id), status=302)
+    assert resp.location == 'http://example.net/backoffice/forms/%s/history/' % formdef.id
+    resp = resp.follow()
+    assert 'Can not display snapshot (Unknown field type [foobar])' in resp
+
+
 def test_workflow_snapshot_browse(pub):
     create_superuser(pub)
     create_role()
diff --git a/wcs/backoffice/snapshots.py b/wcs/backoffice/snapshots.py
index 83d9c8dc..6590cdec 100644
--- a/wcs/backoffice/snapshots.py
+++ b/wcs/backoffice/snapshots.py
@@ -14,14 +14,19 @@
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, see <http://www.gnu.org/licenses/>.
 
-from quixote import get_publisher, get_response, redirect
+from quixote import get_publisher, get_response, redirect, get_session
 from quixote.directory import Directory
 from quixote.html import TemplateIO, htmltext
 
+from wcs.blocks import BlockDef
+from wcs.carddef import CardDef
+from wcs.data_sources import NamedDataSource
+from wcs.formdef import FormDef, FormdefImportError
+from wcs.workflows import Workflow
+from wcs.wscalls import NamedWsCall
 from wcs.qommon import _, errors, misc, template
 from wcs.qommon.backoffice.menu import html_top
 from wcs.qommon.form import Form, RadiobuttonsWidget, StringWidget
-from wcs.qommon.storage import Equal
 
 
 class SnapshotsDirectory(Directory):
@@ -130,8 +135,16 @@ class SnapshotDirectory(Directory):
             return redirect('..')
 
         if form.get_submit() == 'submit':
-            self.snapshot.restore(as_new=bool(action.parse() == 'as-new'))
-            return redirect(self.snapshot.instance.get_admin_url())
+            try:
+                self.snapshot.restore(as_new=bool(action.parse() == 'as-new'))
+            except FormdefImportError as e:
+                reason = _(e.msg) % e.msg_args
+                if e.details:
+                    reason += ' [%s]' % e.details
+                error_msg = _('Can not restore snapshot (%s)') % reason
+                form.set_error('action', error_msg)
+            else:
+                return redirect(self.snapshot.instance.get_admin_url())
 
         get_response().breadcrumb.append(('restore', _('Restore')))
         r = TemplateIO(html=True)
@@ -141,35 +154,44 @@ class SnapshotDirectory(Directory):
 
     @property
     def view(self):
-        from wcs.blocks import BlockDef
-        from wcs.carddef import CardDef
-        from wcs.data_sources import NamedDataSource
-        from wcs.formdef import FormDef
-        from wcs.workflows import Workflow
-        from wcs.wscalls import NamedWsCall
-
         klass = self.snapshot.get_object_class()
+        self.snapshot._check_datasources = False
+        try:
+            instance = self.snapshot.instance
+        except FormdefImportError as e:
+            reason = _(e.msg) % e.msg_args
+            if e.details:
+                reason += ' [%s]' % e.details
+            error_msg = _('Can not display snapshot (%s)') % reason
+            get_session().message = ('error', _(error_msg))
+
+            class RedirectDirectory(Directory):
+                def _q_lookup(sellf, component):
+                    return redirect('../../')
+
+            return RedirectDirectory()
+
         if klass is BlockDef:
             from wcs.admin.blocks import BlockDirectory
 
-            return BlockDirectory(section='forms', objectdef=self.snapshot.instance)
+            return BlockDirectory(section='forms', objectdef=instance)
         if klass is FormDef:
             from wcs.admin.forms import FormDefPage
 
-            return FormDefPage(component='view', instance=self.snapshot.instance)
+            return FormDefPage(component='view', instance=instance)
         if klass is CardDef:
             from wcs.backoffice.cards import CardDefPage
 
-            return CardDefPage(component='view', instance=self.snapshot.instance)
+            return CardDefPage(component='view', instance=instance)
         if klass is Workflow:
             from wcs.admin.workflows import WorkflowPage
 
-            return WorkflowPage(component='view', instance=self.snapshot.instance)
+            return WorkflowPage(component='view', instance=instance)
         if klass is NamedDataSource:
             from wcs.admin.data_sources import NamedDataSourcePage
 
-            return NamedDataSourcePage(component='view', instance=self.snapshot.instance)
+            return NamedDataSourcePage(component='view', instance=instance)
         if klass is NamedWsCall:
             from wcs.admin.wscalls import NamedWsCallPage
 
-            return NamedWsCallPage(component='view', instance=self.snapshot.instance)
+            return NamedWsCallPage(component='view', instance=instance)
diff --git a/wcs/blocks.py b/wcs/blocks.py
index 136c02e0..2325b8f3 100644
--- a/wcs/blocks.py
+++ b/wcs/blocks.py
@@ -116,7 +116,7 @@ class BlockDef(StorableObject):
     def import_from_xml(cls, fd, include_id=False, check_datasources=True):
         try:
             tree = ET.parse(fd)
-        except:
+        except Exception:
             raise ValueError()
         blockdef = cls.import_from_xml_tree(tree, include_id=include_id)
 
@@ -144,7 +144,7 @@ class BlockDef(StorableObject):
         return blockdef
 
     @classmethod
-    def import_from_xml_tree(cls, tree, include_id=False, snapshot=False):
+    def import_from_xml_tree(cls, tree, include_id=False, **kwargs):
         charset = 'utf-8'
         blockdef = cls()
         if tree.find('name') is None or not tree.find('name').text:
diff --git a/wcs/qommon/xml_storage.py b/wcs/qommon/xml_storage.py
index 4e664fec..ea47375e 100644
--- a/wcs/qommon/xml_storage.py
+++ b/wcs/qommon/xml_storage.py
@@ -79,12 +79,12 @@ class XmlStorableObject(StorableObject):
     def import_from_xml(cls, fd, charset=None, include_id=False):
         try:
             tree = ET.parse(fd)
-        except:
+        except Exception:
             raise ValueError()
         return cls.import_from_xml_tree(tree, charset=charset, include_id=include_id)
 
     @classmethod
-    def import_from_xml_tree(cls, tree, include_id=False, charset=None, snapshot=False):
+    def import_from_xml_tree(cls, tree, include_id=False, charset=None, **kwargs):
         if charset is None:
             charset = get_publisher().site_charset
         obj = cls()
diff --git a/wcs/snapshots.py b/wcs/snapshots.py
index 43aff19e..352dcedc 100644
--- a/wcs/snapshots.py
+++ b/wcs/snapshots.py
@@ -85,7 +85,10 @@ class Snapshot:
         if self._instance is None:
             tree = ET.fromstring(self.serialization)
             self._instance = self.get_object_class().import_from_xml_tree(
-                tree, include_id=True, snapshot=True
+                tree,
+                include_id=True,
+                snapshot=True,
+                check_datasources=getattr(self, '_check_datasources', True),
             )
             self._instance.readonly = True
             self._instance.snapshot_object = self
diff --git a/wcs/templates/wcs/backoffice/snapshots.html b/wcs/templates/wcs/backoffice/snapshots.html
index 7c6390e9..8f376e64 100644
--- a/wcs/templates/wcs/backoffice/snapshots.html
+++ b/wcs/templates/wcs/backoffice/snapshots.html
@@ -5,6 +5,8 @@
 <h2>{% trans "History" %}</h2>
 </div>
 
+{{ publisher.get_request.session.display_message|safe }}
+
 {% with snapshots=view.snapshots %}
 {% if snapshots %}
 <div class="section">
-- 
2.30.1

