From cb26ec31c4d2452df53c21f42aed207cbb996944 Mon Sep 17 00:00:00 2001
From: Nicolas ROCHE <nroche@entrouvert.com>
Date: Fri, 16 Oct 2020 19:37:12 +0200
Subject: [PATCH] snapshots: detail status action update comments (#47367)

---
 tests/test_snapshots.py   | 106 ++++++++++++++++++++++++++++++++++++++
 wcs/admin/workflows.py    |  14 +++--
 wcs/wf/export_to_model.py |  10 ++++
 wcs/workflows.py          |  38 ++++++++++++++
 4 files changed, 165 insertions(+), 3 deletions(-)

diff --git a/tests/test_snapshots.py b/tests/test_snapshots.py
index a96a220b..5e156dab 100644
--- a/tests/test_snapshots.py
+++ b/tests/test_snapshots.py
@@ -1,9 +1,10 @@
 import pytest
+from webtest import Upload
 
 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
 
@@ -335,8 +336,113 @@ def test_form_snapshot_save(pub, formdef_with_history):
     for i in range(10, 15):
         formdef.description = 'this is a description (%s)' % i
         formdef.store()
 
     resp = app.get('/backoffice/forms/%s/history/' % formdef_with_history.id)
     assert [x.attrib['class'] for x in resp.pyquery.find('ul.snapshots-list li')] == [
             'new-day', 'collapsed', 'collapsed', 'collapsed', 'collapsed', 'has-label',
             'collapsed', 'collapsed', 'collapsed', 'collapsed', 'collapsed', 'collapsed']
+
+
+def test_workflow_status_item_history_comments(pub):
+    create_superuser(pub)
+    create_role()
+
+    Workflow.wipe()
+    workflow = Workflow(name='test')
+    workflow.add_status(name='baz')
+    workflow.store()
+
+    app = login(get_app(pub))
+    resp = app.get('/backoffice/workflows/1/')
+    resp = resp.click('baz')
+
+    resp.forms[0]['action-formdata-action'] = 'Document Creation'
+    resp = resp.forms[0].submit()
+    resp = resp.follow()
+
+    resp = resp.click('Edit', href='items/1/')
+    resp = resp.forms[0].submit('submit')
+    resp = resp.follow()
+    resp = resp.follow()
+    assert 'No change was introduced' in \
+        resp.html.find('ul', {'class', 'messages'}).find('li', {'class': 'warning'}).text
+
+    resp = resp.click('Edit', href='items/1/')
+    resp.form['model_file$file'] = Upload(
+        'test.odt', b'foobar', 'application/vnd.oasis.opendocument.text')
+    resp = resp.forms[0].submit('submit')
+    resp = resp.follow()
+    resp = resp.follow()
+
+    resp = resp.click('Edit', href='items/1/')
+    resp.form['convert_to_pdf'] = False
+    resp.form['attach_to_history'] = True
+    resp = resp.forms[0].submit('submit')
+    resp = resp.follow()
+    resp = resp.follow()
+
+    resp = resp.click(href='items/1/delete')
+    resp = resp.form.submit('submit')
+
+    resp = app.get('/backoffice/workflows/%s/history/' % workflow.id)
+    comments = [x.text[18:x.text.find('\n')]
+                for x in resp.html.find('ul', {'class': 'snapshots-list'}).find_all('li')]
+    assert comments == [
+        'Deletion of action Document Creation in status "baz"',
+        'Change in action Document Creation in status "baz": set 2 parameters'
+        ' (Convert generated file to PDF, Include generated file in the form history)',
+        'Change in action Document Creation in status "baz": Model set to test.odt',
+        'New action: Document Creation" in status "baz"',
+        '']
+
+
+def test_workflow_status_item_history_comments_label(pub):
+    create_superuser(pub)
+    create_role()
+
+    Workflow.wipe()
+    workflow = Workflow(name='test')
+    workflow.add_status(name='baz')
+    workflow.store()
+
+    app = login(get_app(pub))
+    resp = app.get('/backoffice/workflows/1/')
+    resp = resp.click('baz')
+
+    resp.forms[0]['action-interaction'] = 'Webservice'
+    resp = resp.forms[0].submit()
+    resp = resp.follow()
+
+    resp = resp.click('Edit', href='items/1/')
+    resp.form['label'] = 'foo'
+    resp = resp.forms[0].submit('submit')
+    resp = resp.follow()
+    resp = resp.follow()
+
+    resp = resp.click('Edit', href='items/1/')
+    resp.form['url'] = 'http://example.org'
+    resp = resp.forms[0].submit('submit')
+    resp = resp.follow()
+    resp = resp.follow()
+
+    resp = resp.click('Edit', href='items/1/')
+    resp.form['notify_on_errors'] = True
+    resp.form['record_on_errors'] = False
+    resp = resp.forms[0].submit('submit')
+    resp = resp.follow()
+    resp = resp.follow()
+
+    resp = resp.click(href='items/1/delete')
+    resp = resp.form.submit('submit')
+
+    resp = app.get('/backoffice/workflows/%s/history/' % workflow.id)
+    comments = [x.text[18:x.text.find('\n')]
+                for x in resp.html.find('ul', {'class': 'snapshots-list'}).find_all('li')]
+    assert comments == [
+        'Deletion of action Webservice "foo" in status "baz"',
+        'Change in action Webservice "foo" in status "baz": set 2 parameters'
+        ' (Notify on errors, Record on errors)',
+        'Change in action Webservice "foo" in status "baz": URL set to http://example.org',
+        'Change in action Webservice in status "baz": Label set to foo',
+        'New action: Webservice" in status "baz"',
+        '']
diff --git a/wcs/admin/workflows.py b/wcs/admin/workflows.py
index 65fc3241..89de59c8 100644
--- a/wcs/admin/workflows.py
+++ b/wcs/admin/workflows.py
@@ -289,18 +289,23 @@ class WorkflowItemPage(Directory):
             self.html_top('%s - %s' % (_('Workflow'), self.workflow.name))
             r = TemplateIO(html=True)
             r += htmltext('<h2>%s</h2>') % _(self.item.description)
             r += form.render()
             if self.item.support_substitution_variables:
                 r += get_publisher().substitutions.get_substitution_html_table()
             return r.getvalue()
         else:
+            comment = self.item.comment_submition(form)
+            if not comment:
+                get_session().message = ('warning', _('No change was introduced'))
+                return redirect('..')
+
             self.item.submit_admin_form(form)
-            self.workflow.store(comment=_('Change in action "%s"') % _(self.item.description))
+            self.workflow.store(comment)
             return redirect('..')
 
     def delete(self):
         form = Form(enctype='multipart/form-data')
         form.widgets.append(HtmlWidget('<p>%s</p>' % _('You are about to remove an item.')))
         form.add_submit('delete', _('Delete'))
         form.add_submit('cancel', _('Cancel'))
         if form.get_widget('cancel').parse():
@@ -309,17 +314,17 @@ class WorkflowItemPage(Directory):
             get_response().breadcrumb.append(('delete', _('Delete')))
             self.html_top(title = _('Delete Item'))
             r = TemplateIO(html=True)
             r += htmltext('<h2>%s</h2>') % _('Deleting Item')
             r += form.render()
             return r.getvalue()
         else:
             del self.parent.items[self.parent.items.index(self.item)]
-            self.workflow.store(comment=_('Deletion of action "%s"') % _(self.item.description))
+            self.workflow.store(comment=self.item.comment_deletion())
             return redirect('../../')
 
     def _q_lookup(self, component):
         t = self.item.q_admin_lookup(self.workflow, self.parent, component,
                 self.html_top)
         if t:
             return t
         return Directory._q_lookup(self, component)
@@ -566,17 +571,20 @@ class WorkflowStatusPage(Directory):
         if not form.is_submitted() or form.has_errors():
             get_session().message = ('error', _('Submitted form was not filled properly.'))
             return redirect('.')
 
         for category in ('status-change', 'interaction', 'formdata-action', 'user-action'):
             action_type = form.get_widget('action-%s' % category).parse()
             if action_type:
                 self.status.append_item(action_type)
-                self.workflow.store(comment=_('New action'))
+                description = next((x.description for x in item_classes
+                              if x.key == action_type), None)
+                self.workflow.store(comment=_('New action: %s" in status "%s"') % (
+                    _(description), self.status.name))
                 return redirect('.')
 
         get_session().message = ('error', _('Submitted form was not filled properly.'))
         return redirect('.')
 
 
     def delete(self):
         form = Form(enctype="multipart/form-data")
diff --git a/wcs/wf/export_to_model.py b/wcs/wf/export_to_model.py
index c9c3538e..ff0150dc 100644
--- a/wcs/wf/export_to_model.py
+++ b/wcs/wf/export_to_model.py
@@ -385,16 +385,26 @@ class ExportToModel(WorkflowStatusItem):
                      attrs={
                          'data-dynamic-display-child-of': '%smethod' % prefix,
                          'data-dynamic-display-value': methods.get('interactive'),
                      })
         if 'filename' in parameters:
             form.add(ComputedExpressionWidget, name='%sfilename' % prefix, title=_('File name'),
                      value=self.filename)
 
+    def get_admin_form_changes(self, form):
+        changes = super(ExportToModel, self).get_admin_form_changes(form)
+        for change in changes:
+            if change[0] == 'model_file':
+                for idx in 2, 3:
+                    if hasattr(change[idx], 'base_filename'):
+                        change[idx] = change[idx].base_filename
+            break
+        return changes
+
     def get_model_file_parameter_view_value(self):
         return htmltext('<a href="status/%s/items/%s/?file=model_file">%s</a>') % (
             self.parent.id, self.id, self.model_file.base_filename)
 
     def get_filename(self):
         filename = None
         if self.filename:
             filename = self.compute(self.filename)
diff --git a/wcs/workflows.py b/wcs/workflows.py
index 4d4fc6e1..85d7a9eb 100644
--- a/wcs/workflows.py
+++ b/wcs/workflows.py
@@ -1931,16 +1931,54 @@ class WorkflowStatusItem(XmlSerialisable):
             return '-'
         else:
             return str(value)
 
     def fill_admin_form(self, form):
         for parameter in self.get_parameters():
             self.add_parameters_widgets(form, [parameter])
 
+    def get_admin_form_changes(self, form):
+        changes = []
+        for f in self.get_parameters():
+            widget = form.get_widget(f)
+            if widget:
+                old_value = getattr(self, f)
+                value = widget.parse()
+                if hasattr(self, '%s_parse' % f):
+                    value = getattr(self, '%s_parse' % f)(value)
+                changes.append([f, widget.title, old_value, value])
+        return changes
+
+    def comment_submition(self, form):
+        changes = [x for x in self.get_admin_form_changes(form) if x[2] != x[3]]
+        if not changes:
+            return None
+        if len(changes) == 1:
+            comment = _("%s set to %s") % (changes[0][1], changes[0][3])
+        else:
+            comment = _("set %s parameters (%s)") % (
+                len(changes), ', '.join(x[1] for x in changes))
+        if getattr(self, 'label', None):
+            comment = _('Change in action %s "%s" in status "%s": %s') % (
+                _(self.description), self.label, self.parent.name, comment)
+        else:
+            comment = _('Change in action %s in status "%s": %s') % (
+                _(self.description), self.parent.name, comment)
+        return comment
+
+    def comment_deletion(self):
+        if getattr(self, 'label', None):
+            comment=_('Deletion of action %s "%s" in status "%s"') % (
+                _(self.description), self.label, self.parent.name)
+        else:
+            comment=_('Deletion of action %s in status "%s"') % (
+                _(self.description), self.parent.name)
+        return comment
+
     def submit_admin_form(self, form):
         for f in self.get_parameters():
             widget = form.get_widget(f)
             if widget:
                 value = widget.parse()
                 if hasattr(self, '%s_parse' % f):
                     value = getattr(self, '%s_parse' % f)(value)
                 setattr(self, f, value)
-- 
2.28.0

