From fe11dcc186893654e91de40f03c0e914fe16fa6b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= <fpeters@entrouvert.com>
Date: Sun, 23 Jan 2022 20:07:42 +0100
Subject: [PATCH] general: add soft/hard limits to number of field (#60061)

---
 tests/admin_pages/test_block.py | 34 ++++++++++++++++++++++++
 tests/admin_pages/test_card.py  | 43 ++++++++++++++++++++++++++++++
 tests/admin_pages/test_form.py  | 41 +++++++++++++++++++++-------
 wcs/admin/blocks.py             |  5 ++++
 wcs/admin/fields.py             | 47 ++++++++++++++++++++++++++-------
 wcs/admin/forms.py              |  3 ++-
 wcs/admin/workflows.py          |  9 +++++++
 wcs/backoffice/cards.py         |  9 ++++++-
 wcs/wf/form.py                  |  2 ++
 9 files changed, 172 insertions(+), 21 deletions(-)

diff --git a/tests/admin_pages/test_block.py b/tests/admin_pages/test_block.py
index 1d65cc067..0bacb0b58 100644
--- a/tests/admin_pages/test_block.py
+++ b/tests/admin_pages/test_block.py
@@ -359,3 +359,37 @@ def test_blocks_category(pub):
     resp = resp.forms[0].submit()
     block.refresh_from_storage()
     assert not block.category_id
+
+
+def test_block_edit_field_warnings(pub):
+    create_superuser(pub)
+
+    BlockDef.wipe()
+    blockdef = BlockDef()
+    blockdef.name = 'block title'
+    blockdef.fields = [
+        fields.StringField(id='%d' % i, label='field %d' % i, type='string') for i in range(1, 10)
+    ]
+    blockdef.store()
+
+    app = login(get_app(pub))
+    resp = app.get('/backoffice/forms/blocks/%s/' % blockdef.id)
+    assert 'more than 20 fields' not in resp.text
+
+    blockdef.fields.extend(
+        [fields.StringField(id='%d' % i, label='field %d' % i, type='string') for i in range(10, 21)]
+    )
+    blockdef.store()
+    resp = app.get('/backoffice/forms/blocks/%s/' % blockdef.id)
+    assert 'more than 20 fields' not in resp.text
+    assert '<div id="new-field"><h3>New Field</h3>' in resp.text
+    assert '>Duplicate<' in resp.text
+
+    blockdef.fields.extend(
+        [fields.StringField(id='%d' % i, label='field %d' % i, type='string') for i in range(21, 31)]
+    )
+    blockdef.store()
+    resp = app.get('/backoffice/forms/blocks/%s/' % blockdef.id)
+    assert 'This block of fields contains 30 fields.' in resp.text
+    assert '<div id="new-field"><h3>New Field</h3>' not in resp.text
+    assert '>Duplicate<' not in resp.text
diff --git a/tests/admin_pages/test_card.py b/tests/admin_pages/test_card.py
index ffffca8b8..2ccb28459 100644
--- a/tests/admin_pages/test_card.py
+++ b/tests/admin_pages/test_card.py
@@ -850,3 +850,46 @@ def test_cards_svg(pub):
     assert resp.text.count('<title>card_card_1_1&#45;&gt;card_card_2_2</title>') == 0
     assert resp.text.count('<title>card_card_2_1&#45;&gt;card_card_2_2</title>') == 8
     assert resp.text.count('<title>card_card_3_1&#45;&gt;card_card_3_2</title>') == 0
+
+
+def test_card_edit_field_warnings(pub):
+    create_superuser(pub)
+
+    CardDef.wipe()
+    carddef = CardDef()
+    carddef.name = 'card title'
+    carddef.fields = [
+        fields.StringField(id='%d' % i, label='field %d' % i, type='string') for i in range(1, 10)
+    ]
+    carddef.store()
+
+    app = login(get_app(pub))
+    resp = app.get('/backoffice/cards/%s/fields/' % carddef.id)
+    assert 'more than 200 fields' not in resp.text
+    assert 'first field should be of type "page"' not in resp.text
+
+    carddef.fields.append(fields.PageField(id='1000', label='page', type='page'))
+    carddef.store()
+    resp = app.get('/backoffice/cards/%s/fields/' % carddef.id)
+    assert 'more than 200 fields' not in resp.text
+    assert 'first field should be of type "page"' in resp.text
+    assert '<div id="new-field"><h3>New Field</h3>' in resp.text
+
+    carddef.fields.extend(
+        [fields.StringField(id='%d' % i, label='field %d' % i, type='string') for i in range(10, 210)]
+    )
+    carddef.store()
+    resp = app.get('/backoffice/cards/%s/fields/' % carddef.id)
+    assert 'more than 200 fields' in resp.text
+    assert 'first field should be of type "page"' in resp.text
+    assert '>Duplicate<' in resp.text
+
+    carddef.fields.extend(
+        [fields.StringField(id='%d' % i, label='field %d' % i, type='string') for i in range(210, 310)]
+    )
+    carddef.store()
+    resp = app.get('/backoffice/cards/%s/fields/' % carddef.id)
+    assert 'This card model contains 310 fields.' in resp.text
+    assert 'first field should be of type "page"' in resp.text
+    assert '<div id="new-field"><h3>New Field</h3>' not in resp.text
+    assert '>Duplicate<' not in resp.text
diff --git a/tests/admin_pages/test_form.py b/tests/admin_pages/test_form.py
index a79a19736..fb19f45b5 100644
--- a/tests/admin_pages/test_form.py
+++ b/tests/admin_pages/test_form.py
@@ -2590,26 +2590,47 @@ def test_form_edit_field_warnings(pub):
     formdef.store()
 
     app = login(get_app(pub))
-    resp = app.get('/backoffice/forms/1/')
-    resp = resp.click(href='fields/')
-    assert 'more than 500 fields' not in resp.text
+    resp = app.get('/backoffice/forms/%s/fields/' % formdef.id)
+    assert 'more than 200 fields' not in resp.text
     assert 'first field should be of type "page"' not in resp.text
 
     formdef.fields.append(fields.PageField(id='1000', label='page', type='page'))
     formdef.store()
-    resp = app.get('/backoffice/forms/1/')
-    resp = resp.click(href='fields/', index=0)
-    assert 'more than 500 fields' not in resp.text
+    resp = app.get('/backoffice/forms/%s/fields/' % formdef.id)
+    assert 'more than 200 fields' not in resp.text
     assert 'first field should be of type "page"' in resp.text
+    assert '<div id="new-field"><h3>New Field</h3>' in resp.text
 
     formdef.fields.extend(
-        [fields.StringField(id='%d' % i, label='field %d' % i, type='string') for i in range(10, 510)]
+        [fields.StringField(id='%d' % i, label='field %d' % i, type='string') for i in range(10, 210)]
     )
     formdef.store()
-    resp = app.get('/backoffice/forms/1/')
-    resp = resp.click(href='fields/', index=0)
-    assert 'more than 500 fields' in resp.text
+    resp = app.get('/backoffice/forms/%s/fields/' % formdef.id)
+    assert 'more than 200 fields' in resp.text
     assert 'first field should be of type "page"' in resp.text
+    assert '>Duplicate<' in resp.text
+
+    formdef.fields.extend(
+        [fields.StringField(id='%d' % i, label='field %d' % i, type='string') for i in range(210, 310)]
+    )
+    formdef.store()
+    resp = app.get('/backoffice/forms/%s/fields/' % formdef.id)
+    assert 'This form contains 310 fields.' in resp.text
+    assert 'no new fields can be added.' in resp.text
+    assert 'first field should be of type "page"' in resp.text
+    assert '<div id="new-field"><h3>New Field</h3>' not in resp.text
+    assert '>Duplicate<' not in resp.text
+
+    if not pub.site_options.has_section('options'):
+        pub.site_options.add_section('options')
+    pub.site_options.set('options', 'ignore-hard-limits', 'true')
+    with open(os.path.join(pub.app_dir, 'site-options.cfg'), 'w') as fd:
+        pub.site_options.write(fd)
+
+    resp = app.get('/backoffice/forms/%s/fields/' % formdef.id)
+    assert 'no new fields should be added.' in resp.text
+    assert '<div id="new-field"><h3>New Field</h3>' in resp.text
+    assert '>Duplicate<' in resp.text
 
     FormDef.wipe()
 
diff --git a/wcs/admin/blocks.py b/wcs/admin/blocks.py
index ceee05ed7..1756d90d1 100644
--- a/wcs/admin/blocks.py
+++ b/wcs/admin/blocks.py
@@ -53,6 +53,11 @@ class BlockDirectory(FieldsDirectory):
     support_import = False
     readonly_message = _('This block of fields is readonly.')
 
+    field_count_message = _('This block of fields contains %d fields.')
+    field_over_count_message = _('This block of fields contains more than %d fields.')
+    fields_count_total_soft_limit = 20
+    fields_count_total_hard_limit = 30
+
     def __init__(self, section='forms', *args, **kwargs):
         if kwargs.pop('component', None):  # snapshot
             kwargs['objectdef'] = kwargs.pop('instance')
diff --git a/wcs/admin/fields.py b/wcs/admin/fields.py
index f57c0cf59..b24dc4f43 100644
--- a/wcs/admin/fields.py
+++ b/wcs/admin/fields.py
@@ -17,7 +17,7 @@
 import copy
 import json
 
-from quixote import get_request, get_response, get_session, redirect
+from quixote import get_publisher, get_request, get_response, get_session, redirect
 from quixote.directory import Directory
 from quixote.html import TemplateIO, htmlescape, htmltext
 
@@ -279,8 +279,12 @@ class FieldsDirectory(Directory):
     page_id = None
     field_var_prefix = '..._'
     readonly_message = _('The fields are readonly.')
+    field_count_message = _('This form contains %d fields.')
+    field_over_count_message = _('This form contains more than %d fields.')
 
     support_import = True
+    fields_count_total_soft_limit = 200
+    fields_count_total_hard_limit = 300
 
     def html_top(self, title, *args, **kwargs):
         html_top(self.section, title, *args, **kwargs)
@@ -309,16 +313,23 @@ class FieldsDirectory(Directory):
         r = TemplateIO(html=True)
 
         r += self.index_top()
+        ignore_hard_limits = get_publisher().has_site_option('ignore-hard-limits')
 
         if self.objectdef.fields:
-            if len(self.objectdef.fields) > 500:
+            if len(self.objectdef.fields) >= self.fields_count_total_hard_limit:
                 r += htmltext('<div class="errornotice">')
-                r += htmltext(
-                    _(
-                        'This form contains more than 500 fields. '
-                        'It is close to the database limits and no new fields should be added.'
-                    )
-                )
+                r += htmltext(self.field_count_message % len(self.objectdef.fields))
+                r += htmltext(' ')
+                if ignore_hard_limits:
+                    r += htmltext(_('It is over system limits and no new fields should be added.'))
+                else:
+                    r += htmltext(_('It is over system limits and no new fields can be added.'))
+                r += htmltext('</div>')
+            elif len(self.objectdef.fields) > self.fields_count_total_soft_limit:
+                r += htmltext('<div class="errornotice">')
+                r += htmltext(self.field_over_count_message % self.fields_count_total_soft_limit)
+                r += htmltext(' ')
+                r += htmltext(_('It is close to the system limits and no new fields should be added.'))
                 r += htmltext('</div>')
 
             if [x for x in self.objectdef.fields if x.type == 'page']:
@@ -404,7 +415,10 @@ class FieldsDirectory(Directory):
                         )
                     r += command_icon('%s/' % field.id, 'edit')
                 if not self.objectdef.is_readonly():
-                    r += command_icon('%s/duplicate' % field.id, 'duplicate', popup=(field.type == 'page'))
+                    if len(self.objectdef.fields) < self.fields_count_total_hard_limit or ignore_hard_limits:
+                        r += command_icon(
+                            '%s/duplicate' % field.id, 'duplicate', popup=(field.type == 'page')
+                        )
                     r += command_icon('%s/delete' % field.id, 'remove', popup=True)
                 r += htmltext('</p></li>')
             r += htmltext('</ul>')
@@ -424,6 +438,21 @@ class FieldsDirectory(Directory):
 
     def get_new_field_form(self, page_id):
         r = TemplateIO(html=True)
+        ignore_hard_limits = get_publisher().has_site_option('ignore-hard-limits')
+
+        if len(self.objectdef.fields) >= self.fields_count_total_hard_limit:
+            if ignore_hard_limits:
+                r += htmltext('<div class="errornotice"><p>%s %s</p></div>') % (
+                    self.field_count_message % self.fields_count_total_hard_limit,
+                    _('It is over system limits and no new fields should be added.'),
+                )
+            else:
+                r += htmltext('<div class="errornotice"><p>%s %s</p></div>') % (
+                    self.field_count_message % self.fields_count_total_hard_limit,
+                    _('It is over system limits and no new fields can be added.'),
+                )
+                return r.getvalue()
+
         r += htmltext('<div id="new-field">')
         r += htmltext('<h3>%s</h3>') % _('New Field')
         get_request().form = None  # ignore the eventual ?page=x
diff --git a/wcs/admin/forms.py b/wcs/admin/forms.py
index 03bcc2ec4..eda86fc0b 100644
--- a/wcs/admin/forms.py
+++ b/wcs/admin/forms.py
@@ -628,6 +628,7 @@ class FormDefPage(Directory):
     formdef_default_workflow = '_default'
     section = 'forms'
     options_directory_class = OptionsDirectory
+    fields_directory_class = AdminFieldsDirectory
 
     delete_message = _('You are about to irrevocably delete this form.')
     delete_title = _('Deleting Form:')
@@ -647,7 +648,7 @@ class FormDefPage(Directory):
             raise TraversalError()
         self.formdefui = self.formdef_ui_class(self.formdef)
         get_response().breadcrumb.append((component + '/', self.formdef.name))
-        self.fields = AdminFieldsDirectory(self.formdef)
+        self.fields = self.fields_directory_class(self.formdef)
         self.fields.html_top = self.html_top
         self.role = WorkflowRoleDirectory(self.formdef)
         self.role.html_top = self.html_top
diff --git a/wcs/admin/workflows.py b/wcs/admin/workflows.py
index 402192f93..f5f09adbe 100644
--- a/wcs/admin/workflows.py
+++ b/wcs/admin/workflows.py
@@ -1051,6 +1051,11 @@ class WorkflowVariablesFieldsDirectory(FieldsDirectory):
     field_var_prefix = 'form_option_'
     readonly_message = _('This workflow is readonly.')
 
+    field_count_message = _('This workflow contains %d variables.')
+    field_over_count_message = _('This workflow contains more than %d variables.')
+    fields_count_total_soft_limit = 20
+    fields_count_total_hard_limit = 30
+
     def index_top(self):
         r = TemplateIO(html=True)
         r += htmltext('<h2>%s - %s - %s</h2>') % (_('Workflow'), self.objectdef.name, _('Variables'))
@@ -1073,6 +1078,10 @@ class WorkflowBackofficeFieldsDirectory(FieldsDirectory):
     blacklisted_attributes = ['condition']
     field_var_prefix = 'form_var_'
     readonly_message = _('This workflow is readonly.')
+    field_count_message = _('This workflow contains %d backoffice fields.')
+    field_over_count_message = _('This workflow contains more than %d backoffice fields.')
+    fields_count_total_soft_limit = 30
+    fields_count_total_hard_limit = 45
 
     def index_top(self):
         r = TemplateIO(html=True)
diff --git a/wcs/backoffice/cards.py b/wcs/backoffice/cards.py
index 99780012d..88d31f012 100644
--- a/wcs/backoffice/cards.py
+++ b/wcs/backoffice/cards.py
@@ -19,7 +19,7 @@ from quixote.html import TemplateIO, htmltext
 
 from wcs.admin import utils
 from wcs.admin.categories import CardDefCategoriesDirectory
-from wcs.admin.forms import FormDefPage, FormDefUI, FormsDirectory, OptionsDirectory
+from wcs.admin.forms import AdminFieldsDirectory, FormDefPage, FormDefUI, FormsDirectory, OptionsDirectory
 from wcs.carddef import CardDef, get_cards_graph
 from wcs.categories import CardDefCategory
 from wcs.workflows import Workflow
@@ -41,6 +41,12 @@ class CardDefOptionsDirectory(OptionsDirectory):
     section = 'cards'
 
 
+class CardDefFieldsDirectory(AdminFieldsDirectory):
+    field_count_message = _('This card model contains %d fields.')
+    field_over_count_message = _('This card model contains more than %d fields.')
+    readonly_message = _('This card model is readonly.')
+
+
 class CardDefPage(FormDefPage):
     formdef_class = CardDef
     formdef_export_prefix = 'card'
@@ -49,6 +55,7 @@ class CardDefPage(FormDefPage):
     section = 'cards'
 
     options_directory_class = CardDefOptionsDirectory
+    fields_directory_class = CardDefFieldsDirectory
 
     delete_message = _('You are about to irrevocably delete this card model.')
     delete_title = _('Deleting Card Model:')
diff --git a/wcs/wf/form.py b/wcs/wf/form.py
index 804a691cf..2d31a7f99 100644
--- a/wcs/wf/form.py
+++ b/wcs/wf/form.py
@@ -100,6 +100,8 @@ class WorkflowFormFieldsDirectory(FieldsDirectory):
     support_import = False
     blacklisted_types = ['page', 'computed']
     field_def_page_class = WorkflowFormFieldDefPage
+    fields_count_total_soft_limit = 20
+    fields_count_total_hard_limit = 30
 
 
 class FormWorkflowStatusItem(WorkflowStatusItem):
-- 
2.35.1

